(sorry, once again I don't remember which is the appropriate issue queue for this)

Some methods in Drupal have a parameter that controls whether an exception can be thrown, or null is returned on failure.

Example from Drupal\Component\Plugin\Discovery\DiscoveryInterface:

  /**
   * Gets a specific plugin definition.
   *
   * @param string $plugin_id
   *   A plugin id.
   * @param bool $exception_on_invalid
   *   (optional) If TRUE, an invalid plugin ID will throw an exception.
   *
   * @return mixed
   *   A plugin definition, or NULL if the plugin ID is invalid and
   *   $exception_on_invalid is FALSE.
   *
   * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
   *   Thrown if $plugin_id is invalid and $exception_on_invalid is TRUE.
   */
  public function getDefinition($plugin_id, $exception_on_invalid = TRUE);

Such methods confuse the IDE and static code analysis tools.
Because the IDE does not understand the meaning of the second parameter, one still needs to wrap the entire thing in try/catch to prevent inspection warnings.

I propose, as a policy, that whenever we introduce new methods, there should be distinct methods with and without exception.

E.g. if we were to rewrite Drupal\Component\Plugin\Discovery\DiscoveryInterface (which I am not proposing at all):

  /**
   * Gets a specific plugin definition or NULL.
   *
   * @param string $plugin_id
   *   A plugin id.
   *
   * @return mixed
   *   A plugin definition, or NULL if the plugin ID is invalid.
   */
  public function getDefinition($plugin_id);

  /**
   * Gets a specific plugin definition, or throws an exception.
   *
   * @param string $plugin_id
   *   A plugin id.
   *
   * @return mixed
   *   A plugin definition, if the $plugin_id is valid (exception otherwise).
   *
   * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
   *   Thrown if $plugin_id is invalid.
   */
  public function requireDefinition($plugin_id);

For my taste, we could name the first method getDefinitionOrNull() instead. But I think to remember from previous discussions that people find this awkward, so I am fine with getDefinition().

The question whether this is a good idea or not applies beyond Drupal or PHP, so I started this stackexchange question: https://softwareengineering.stackexchange.com/questions/362900/parameter...

Comments

donquixote created an issue. See original summary.

donquixote’s picture

The stackexchange discussion turned out quite interesting.

Arseni Mourzenko:

https://softwareengineering.stackexchange.com/questions/362900/parameter...

Ewan:

I like the bool TryMethodName(out returnValue) approach.

Here we learn about a pattern in C# / .NET, which in PHP would be like using a by-reference parameter for the result, whereas the return value would be true/false for success or failure. I am not advocating for this here, it just looks interesting.

Polygnome:

While I have to work in PHP and am quite proficient, its simply a bad language

Valuable lesson here..

Voo:

Providing two methods instead of thinking about what kind of use case we're dealing with, is the easy way out: Don't think about what you're doing, instead put the responsibility on all the users of your API. Sure it works, but that's not the hallmark of good API design (or any design for the matter. Read e.g. Joel's take on this.

This is an interesting point.
Why would we want to provide the two cases? Why can we not make a decision which version we want to support?
I do have some thoughts on this on my own, which I posted over there.

supercat:

While other people suggest otherwise, [..] having an entry point that can serve both usage patterns will avoid code duplication in wrapper methods.

This is true.
Wrappers or decorators become more cluttered by having two distinct methods.
But zero upvotes, at the time I write this comment.

Conclusion

Some people question whether we should ever support two behaviors (exception vs null).

But a clear tendency seems to be that if we support two behaviors, it should be in separate methods, and not in the same method, for the sake of a more rigid signature.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.