[policy, no patch] Distinct methods with and without exception?

Created on 23 December 2017, over 7 years ago
Updated 27 May 2025, 11 days ago

(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...

🌱 Plan
Status

Postponed: needs info

Version

11.0 🔥

Component

base system

Created by

🇩🇪Germany donquixote

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇳🇿New Zealand quietone

    There has been no discussion here for 8 1/2 years suggesting there is no interest in this and that it can be closed.

    Does anyone else support this idea? If there is support, add a comment. It would also help to update the issue summary using the standard issue template .

    I am setting the status to Postponed (maintainer needs more info. If we don't receive additional information to help with the issue, it may be closed after three months.

    Changing title per Special titles .

  • 🇩🇪Germany donquixote

    I somehow doubt that this is going anywhere.
    Lets see if there are any responses from others, otherwise yeah it can go its path to eternal sleep.

Production build 0.71.5 2024