On Thu, Feb 19, 2015 at 6:14 PM, Trevor Suarez <ric...@gmail.com> wrote:
> 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
>
We thought about "Throwable" or "Catchable" interface, but this change
would require more changes and will make more BC breaks.
Thanks. Dmitry.
>
> - 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.
>>
>
