PHPLIB-1185: Add new methods to get database and collection instances #1494
Conversation
| * @param string $databaseName Name of the database containing the collection | ||
| * @param string $collectionName Name of the collection to select | ||
| * @param array $options Collection constructor options | ||
| * @throws InvalidArgumentException for parameter/option parsing errors |
There was a problem hiding this comment.
I suppose you want the doc block for the note about "does not exist", but the parameter annotations here have no value. The code is self-documenting.
There was a problem hiding this comment.
Does it helps static analysis to tell which kind of exception can be thrown?
There was a problem hiding this comment.
Perhaps, but since there is no throws within this method I would assume that tooling would inherit from doc blocks of any referenced methods, which would have @throws. I'll defer to you guys, though.
There was a problem hiding this comment.
I left @throws in for the time being, since they are included in all other methods as well. I'm on the fence about the usefulness of them, I can see them being useful when coupled with static analysis, but since we don't have anything to enforce correctness of @throws statements, we may reconsider having them for methods that don't directly throw any exceptions.
That said, I did remove the @param tags as the parameter names are self-explanatory.
alcaeus
force-pushed
the
phplib-1185-get-database-collection
branch
from
e33b967
to
ef20376
Compare
PHPLIB-1185
This PR adds the following new methods:
Client::getDatabase, changedClient::selectDatabaseandClient::__getto proxy to this methodClient::getCollection, changedClient::selectCollectionto proxy to this methodDatabase::getCollection, changedDatabase::selectCollectionandDatabase::__getto proxy to this methodSince they are widely used, I did not add any deprecation warnings, nor did I add any language to the documentation to indicate that they are deprecated. I left the tests as they are, as the old methods were changed to proxy to the new methods, so we are implicitly testing them. I can duplicate the existing tests, but I don't think this is necessary.
As a follow-up to this PR, we should update the documentation to use the new
get(Database|Collection)methods, starting in 2.0 (as we only maintain a single documentation for 1.x, so users would have to figure out which method is correct for their version).