I think that structure makes sense Dmitry.
Though, just a bit on naming here (I know, its not a big deal, and naming
is hard, haha):
I think that naming the new parent exception something like "Throwable" or
"Catchable" (as Nikita previously suggested) would be a bit more concise in
meaning than "BaseException". You may not have even meant that name as a
formal proposal, but I figured I'd weigh in regardless. :P
- Trevor
On Thu Feb 19 2015 at 4:55:38 AM Dmitry Stogov <dmi...@zend.com> wrote:
> 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.
>
