> -----Original Message-----
> From: Derick Rethans [mailto:der...@php.net]
> Sent: Monday, April 18, 2016 6:18 PM
> To: Marco Pivetta <ocram...@gmail.com>
> Cc: Dan Ackroyd <dan...@basereality.com>; Bronisław Białek
> <afte...@gmail.com>; PHP internals <internals@lists.php.net>
> Subject: Re: [PHP-DEV] [VOTE] Catching Multiple Exception Types
>
> On Mon, 18 Apr 2016, Marco Pivetta wrote:
>
> > Heya,
> >
> > I voted "NO" due to previous discussion. TL;DR: this is FAR off the
> > 80/20 use-case for a language syntax change.
>
> I'm with you on this one.
>
> > C&P from my answer elsewhere:
> >
> > The following typical example is something REALLY rare, and requiring
> > a parser change for it seems excessive:
> >
> > try {
> > // ...
> > } catch (InvalidArgumentException $e) {
> > // same handling
> > } catch (PDOException $e) {
> > // same handling
> > } catch (BadMethodCallException $e) {
> > // same handling
> > }
> >
> > These 3 exceptions usually result in separate handling anyway. If same
> > handling is needed, you can as usual extract a private method (if you
> > are in a class) and deal with it there:
> >
> > try {
> > // ...
> > } catch (InvalidArgumentException $e) {
> > $this->sameHandling($e);
> > } catch (PDOException $e) {
> > $this->sameHandling($e);
> > } catch (BadMethodCallException $e) {
> > $this->sameHandling($e);
> > }
> >
> > private function sameHandling(Throwable $caught)
> > {
> > // same handling
> > }
>
> Even this is a scary way to do it. As you don't know which exception class you
> have, how would you know how to handle each separately.
In reality, you often don't truly 'handle' exceptions at all. Different
exception types are very often just a slightly fancier and easier way to return
errors, and it's actually very common and perfectly reasonable to handle groups
of exceptions in an identical way. It's also extremely common to see exception
handlers that don't pay any attention to the exception object itself. In fact,
glancing through the Zend Framework 2 codebase and the Drupal 8 codebase, it
seems that the exception object is rarely ever used.
In Drupal 8, many catch blocks simply resort to return FALSE or TRUE. For
example:
try {
return $router->match($path);
}
catch (ResourceNotFoundException $e) {
return FALSE;
}
catch (ParamNotConvertedException $e) {
return FALSE;
}
catch (AccessDeniedHttpException $e) {
return FALSE;
}
catch (MethodNotAllowedException $e) {
return FALSE;
}
}
Note that this is in fact different from catch (Exception $e), and I'm guessing
the author intentionally did not want to catch an exception the type of which
she/he weren't expecting to get - and instead, have it propagate upwards.
With this RFC approved, it would become:
try {
return $router->match($path);
}
catch
(ResourceNotFoundException|ParamNotConvertedException|AccessDeniedHttpException|MethodNotAllowedException
$e) {
return FALSE;
}
Which is much, much nicer and readable. And I found this in the ZF2 codebase:
} catch (DiRuntimeException $e) {
if ($methodRequirementType & self::RESOLVE_STRICT) {
//finally ( be aware to do at the end of flow)
array_pop($this->currentDependencies);
if (isset($alias)) {
array_pop($this->currentAliasDependenencies);
}
// if this item was marked strict,
// plus it cannot be resolve, and no value exist, bail
out
throw new Exception\MissingPropertyException(
sprintf(
'Missing %s for parameter ' . $name . ' for '
. $class . '::' . $method,
(($value[0] === null) ? 'value' :
'instance/object')
),
$e->getCode(),
$e
);
} else {
//finally ( be aware to do at the end of flow)
array_pop($this->currentDependencies);
if (isset($alias)) {
array_pop($this->currentAliasDependenencies);
}
return false;
}
} catch (ServiceManagerException $e) {
// Zend\ServiceManager\Exception\ServiceNotCreatedException
if ($methodRequirementType & self::RESOLVE_STRICT) {
//finally ( be aware to do at the end of flow)
array_pop($this->currentDependencies);
if (isset($alias)) {
array_pop($this->currentAliasDependenencies);
}
// if this item was marked strict,
// plus it cannot be resolve, and no value exist, bail
out
throw new Exception\MissingPropertyException(
sprintf(
'Missing %s for parameter ' . $name . ' for '
. $class . '::' . $method,
(($value[0] === null) ? 'value' :
'instance/object')
),
$e->getCode(),
$e
);
} else {
//finally ( be aware to do at the end of flow)
array_pop($this->currentDependencies);
if (isset($alias)) {
array_pop($this->currentAliasDependenencies);
}
return false;
}
}
To save everyone some time, the two code blocks for the two exception types are
completely identical. I'm not sure why it wasn't function()alized, but
regardless, it's another case where the new proposed feature would make perfect
sense.
I think saying that identical handling for multiple exception types is bad or
is grounds for an urgent code review is fairly disconnected from reality in
many if not most cases.
Zeev
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
