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