On Wed, Feb 18, 2015 at 10:30 PM, Dan Ackroyd <dan...@basereality.com>
wrote:
> On 13 February 2015 at 23:25, Nikita Popov <nikita....@gmail.com> wrote:
> > Subclassing: Should there be more specific subclasses of EngineException
> > for particular errors?
>
> It's not obvious that any subclasses would be useful. However using
> the code to specify the exact type of error, rather than having to
> inspect the message would be good.
>
> > Should EngineException inherit from Exception (and as such be
> > subject to catch(Exception)) or should we introduce some kind
> > of special super-class that is not caught by default
>
> Even ignoring the BC problem with having EngineExceptions extending
> Exception, I think the EngineException needs to be in a different
> hierarchy to Exception to be able to write reasonable code in the
> future
>
> Without having EngineException in a separate hierarchy of exceptions,
> the code below will catch exceptions where the data is 'ok' but there
> was a problem with the code, and continue to process items. This is
> almost certainly not the correct behaviour when an EngineException has
> been encountered.
>
> interface Service {
> function foo($item);
> }
>
>
> function processData(array $itemsToProcess, service $service) {
> foreach ($itemsToProcess as $item) {
> try {
> $service->foo($item);
> }
> // Because $service can throw an Exception that is specific to the
> // implementation we have to catch \Exception, unless we are going
> // to list all possible implementation specific exception types here.
> // That would be a subtle case of strong coupling, and when a new
> // implementation is made the new exception type would need to
> // be added here.
> catch(\Exception $e) {
> // item was not processable but PHP engine is OK.
> $item->markAsErrored();
> //Go on to process the next item
> }
> }
> }
>
>
> To avoid having EngineExceptions in a separate hierarchy, this
> function could be converted to:
>
> function processData(array $itemsToProcess, service $service) {
> foreach ($itemsToProcess as $item) {
> try {
> $service->foo($item);
> }
> catch(\EngineException $ee) {
> //PHP engine is not stable - lets get out of here.
> throw $ee; //or throw new ProcessException($ee)
> }
> catch(\Exception $e) {
> $item->markAsErrored();
> }
> }
> }
>
> However that is bad as i)it's boiler plate to do the correct behaviour
> ii) you have to remember to do that everywhere. Having to remember to
> do the correct thing, is going to lead to people forgetting.
>
> It will still be necessary to catch all types of Exception in a single
> catch block i.e. at the top level of a script to prevent exceptions
> being shown to the end user. This could be made easier by having a
> common super class for Exception and EngineException. However having
> one try block that is required to have multiple catch statements to
> catch all different types of exception is not that much of a burden:
>
> try {
> runApp();
> }
> catch(EngineException $e) {
> handleException($ee);
> }
> catch(Exception $e) {
> handleException($e);
> }
>
> As that would be the only place it would be required to catch both types.
>
> TL:DR EngineException needs to not extend Exception, whether we need a
> super class is not as clear.
>
> cheers
> Dan
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
I think we may introduce the following hierarchy
abstarct class BaseException {
}
class Exception extends BaseException {
}
class EngineException extends BaseException {
}
the existing code that caught Exception is going to be unaffected.
New code may decide to catch engine exception in separate catch block
(using EngineException) or in single block (using BaseException)
Thanks. Dmitry.
