Skip to content

PHPLIB-980: Make uri parameter for MongoDB\Client::__construct nullable #986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Oct 4, 2022

PHPLIB-980

I decided to only make the $uri parameter nullable. While we could widen the types of both options parameters (constructors are excluded from contravariance checks), I believe this is contra-productive. The signature of MongoDB\Manager historically allowed null, but it's preferable to not allow null and instead default to an empty array. This is something I'd like to change in version 2.0 of the extension.

This also becomes a non-issue starting with PHP 8, as users can use named parameters if they want to only pass a single option:

$client = new Client(driverOptions: ['foo' => 'bar']);

This makes using null as an easy default value easier.

For convenience and documentation purposes, I've also extracted the default URI into a public constant.

@alcaeus alcaeus requested a review from jmikola October 4, 2022 07:21
@alcaeus alcaeus self-assigned this Oct 4, 2022
@@ -19,7 +19,7 @@ Definition

.. code-block:: php

function __construct($uri = 'mongodb://127.0.0.1/', array $uriOptions = [], array $driverOptions = [])
function __construct(?string $uri = null, array $uriOptions = [], array $driverOptions = [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $uri docs on https://www.mongodb.com/docs/php-library/current/reference/method/MongoDBClient__construct/ already state:

Defaults to "mongodb://127.0.0.1:27017" if unspecified.

So there's no need to change anything else on this page. I also don't think there's any need to document the constant you introduced.

@alcaeus alcaeus force-pushed the phplib-980-ensure-compatible-signatures branch from e5c93ae to da207f4 Compare October 6, 2022 06:08
@alcaeus alcaeus merged commit 1cc953a into mongodb:master Oct 6, 2022
@alcaeus alcaeus deleted the phplib-980-ensure-compatible-signatures branch October 6, 2022 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants