symfony / symfony-docs

The Symfony documentation
https://symfony.com/doc
Other
2.15k stars 5.1k forks source link

Update optional_dependencies.rst #19954

Closed hbgamra closed 1 month ago

hbgamra commented 1 month ago

Update optional Dependency param

xabbuh commented 1 month ago

What is the reason for this change? I mean why should one pass null to a setLogger() method?

hbgamra commented 1 month ago

You are right, but ultimately the parameter can accept null and is likely to be null

xabbuh commented 1 month ago

Why is that likely? I’d argue that if you don’t want to set a logger, don’t call this method. For now, I don’t see a reason why we should change the docs, but you can of course do it differently in your code if you prefer to.

hbgamra commented 1 month ago

You raise a valid point. Calling an optional method with an optional parameter may seem redundant and it's not good

javiereguiluz commented 1 month ago

This example is part of this section:

Ignoring Missing Dependencies
-----------------------------

The behavior of ignoring missing dependencies is the same as the "null" behavior
except when used within a method call, in which case the method call itself
will be removed.

[...]

So, in this example, the argument of public function setLogger(LoggerInterface $logger): void doesn't have to be nullable because, if $logger is null, the method will never be called. So, the method will never receive a null argument.

If you use this method in other parts of the app, then the argument could be nullable depending on your code. But, for the code example of this doc section, is not strictly needed. That's why I'0m closing this PR as won't fix. @hbgamra if you think I missed something, please tell me and we'll reopen this. Thanks!