How to handle Exceptions for product labels

[dennis-fedco]

I have some code that is run for multiple product lines, A, B, C. 

 

I've coded up a factory that takes product line, some data, and produces a label for that product.  Currently something like this:

//pseudo-codish
foreach(..)
{
    try
    {
        $x->getLabel($productLine);
    }
    catch(Exception $e)
    {
        Log::write("can't create label");
    }
}

getLabel() takes a $productLine, which is then sent to a factory class, and if it's a new product line, the factory throws an exception as it doesn't know how to handle that product line.

 

I am thinking that why am I writing exception for this?  Why not

1. get rid of exception throwing and exception handling code
2. just move that Log::write() line to the place where I am currently throwing the exception (not shown)

 

My question is thus -- Do I need to use Exception Handling in this case or do I remove Exceptions from the code? 

 

The end result is kind of the same.  Except, without exception, (without doing any extra coding work to return parameters), I will essentially be unable to direct the flow of code at the higher level (such as pictured above), based on whether the product line was known or unknown to the factory.

[requinix]

You never need to use exceptions. There's always an alternative. The question is whether exceptions are a better choice.

 

First, exceptions versus logging. I see two potential problems that need to be accounted for:

1. The factory cannot produce the type of thing that was requested of it. If you log a message here then it should be about how the factory could not do its job.

2. That code you've posted can't get a label and thus cannot do whatever it was trying to do. If you log a message here then it should be about how the code could not do its job.

 

Personally I would probably do both. The first one is urgent as it means somebody didn't write some code for the new product and that needs to be fixed as soon as possible. The second one may or may not be urgent, but at the very least a failure probably means that somebody needs to take some sort of action. It's possible for the factory to fail when used elsewhere, and it's possible for this code to fail while the factory succeeds, so neither is dependent upon the other.

 

 

As for whether to use exceptions, that's a question without a definite answer.

PHP traditionally doesn't use exceptions much (though that will start changing drastically with PHP 7!) and errors are propagated by using null and false. Your factory could do the same: return null for failure, and make sure code calling it (and potentially code calling that, and so on) is aware that the factory may return null. So no writing

Factory::createTheThing($data)->method();

unless you are okay with your code potentially crashing. Which you shouldn't be.

 

Exceptions are a bit nicer in that your code can, for the most part, assume success in cases where it doesn't need to handle error. For example, getLabel() may not need to care if the factory fails, however something somewhere should care (such as your code there) or else the exception will be handled by PHP as a fatal error.

It's also much, much easier to pass along different types of failure. If the factory returns null then you probably don't know why and it could have been one of many types of errors, however you can differentiate a ProductClassNotFoundException from a InvalidProductClassException.

 

Personally I use both but it's hard to explain the reasoning I use for both. (One of those "it just makes more sense this way" things.)

[dennis-fedco]

thanks..

 

... in my case labels are only to be produced for certain product lines, so for example we have vetted lables for A and B only, but not C.

my code (above) is placed into existing code that exposes my code to all product lines.  In other words, my code sees all product lines but must only produce labels when it sees A or B (and new lines needing labels may be introduced later)

 

So if my label-generating-code fails for reason of "no label defined for existing product line X", it is not big deal, meaning "all is well" and nothing should be looked into.  Otherwise, for other errors it may be good to log a message.

 

One thing exception handling does for me is it has a standard way to propagate the error message from a Leaf Function of the code, up the chain, to some parent function that has a catch block. Thereby saving me the work and saving me the cluttering up of my code with trying to pass such error message up the chain myself.

 

I see your point about differentiating between Factory failing or my code failing, and I think I am going to keep the exception just becaues I already written it that way.  But truly I do not think it is something that needs exception as somewhere I have read an advice that said Only use exceptions for exceptionary situation.  I am tempted to say that in my case, my use case is not very exceptional.  And still, despite this, using Exceptions makes my life easier in this case.  If I remove exceptions, I lose the up-the-chain ability and will just end up with some logging messages.

 

So in other words, I am still in "what am I doing" stage.

[requinix]

Only use exceptions for exceptionary situation.

A reasonable statement, but subject to interpretation. My interpretation is not "don't use exceptions at all" but rather "don't propagate normal-use exceptions".

 

For your code there it is not necessarily "exceptional", however the factory doesn't know that. And it's the one throwing the exception. All it knows is that it was told to create something that it can't do. As such it should throw an exception because in the general case it should only be called when it is able to do its job. What you're doing is using that as a form of validation: instead of writing your code such that it only tries to create labels for things that can have labels, you're pretending that everything can and then waiting for something to tell you that you're wrong.

 

Catching all exceptions is a bad practice, and violates the "don't propagate normal-use exceptions" interpretation*, so

try {
	$x->getLabel($productLine);
} catch (NoLabelForProductLineException $e) {
	// ignore
}

Or to bypass the exception issue entirely,

foreach ... {
	if (!$x->hasLabel($productLine)) {
		continue;
	}

	$x->getLabel($productLine);
}

* Well, the implied "...but do propagate unexpected exceptions" part of it. Edited May 27, 2015 by requinix