Split API classes and interfaces #2
Conversation
| } | ||
|
|
||
| // Note: this expects consistent command response documents from the server | ||
| static public function createFromCommandResult(CommandResult $result) |
There was a problem hiding this comment.
These factory methods may not even be necessary, as the userland layer can simply interpret the command response and construct the cursors outright.
There was a problem hiding this comment.
I agree, don't add this if it's not necessary... but, alternatively the constructor could just accept Server and CommandResult instead?
There was a problem hiding this comment.
As discussed in video chat, the constructor cannot take a CommandResult directly, since that leaves us with now way to create command cursors from parallelCollectionScan result documents. We agreed to remove this factory method completely.
| * @param string $db Database name | ||
| * @param array $options Connection options | ||
| */ | ||
| public function __construct(array $servers, $db, $options) |
There was a problem hiding this comment.
@bjori: What was the intention of the database and connection options here? If we have Server instances, aren't the connections already established?
There was a problem hiding this comment.
We discussed that both arguments should be removed, and we're be swapping the constructor with fromDsn(), since DSN-based construction is the default convention. Servers can be manually constructed with their own options, and we'll have a general factory method to create a Manager with a specific collection of servers.
Some highlights: fix class/interface syntax; additional class/method docs; Manager convenience methods; more value objects.
| /** | ||
| * Value object for a database command document. | ||
| */ | ||
| final class Command |
There was a problem hiding this comment.
This and Query do not have getter methods, since I assume their data will be accessed internally. Users shouldn't have any reason to read these objects after creating them.
| * | ||
| * @param string $namespace | ||
| * @param array|object $document Document to insert | ||
| * @param array $writeOptions Write concern options |
There was a problem hiding this comment.
I went back to using an array in lieu of WriteOptions class, since the set of options is really not a value object. The write options array for the three single-operation convenience methods is comparable to the options used in constructing the WriteBatch classes, except the WriteBatch classes would require an ordered option as well (akin to what we currently have in MongoWriteBatch).
Split API classes and interfaces
Split previous API files into many more classes and interfaces. Manager and Server APIs were left as-is.
It may be helpful to create a class hierarchy/diagram.