Exceptions and model logic

Rails
1

Hi,
When is it correct to raise an exception? I have an order model and in
my order controller I have an action to mark the order as shipped. I
want
to ensure that if the order has been cancelled i.e. its status is
“cancelled”,
that it cannot be shipped.

I was tempted to do this with validations, but I think they are more for
data validation rather than this sort of logic.

I thought about doing something like this:

controller:

def mark_as_shipped
order = Order.find(params[:id])
begin
order.mark_as_shipped
flash[:notice] = “Order has been updated”
rescue OrderCancelledException
flash[:notice] = “Order has been cancelled”
end
redirect_to :action => ‘list’
end

model:

class Order << AR:B
def mark_as_shipped
if self.status == ‘cancelled’
raise OrderCancelledException
end
# do shipping stuff
save
end
end

What do you think?

Cheers,
Jordan

2

Ouch, this has always been a touch subject for me as well. I’ve never
quite felt comfortable with a philosophy on when to use Exceptions and
when to not use them. Perhaps its just one of the many brain blocks I
have been blessed with.

Perhaps these 5 points may be of some help:

  • Exceptions are expensive to use. They can, in some cases materially,
    affect the performance of your applicaiton. Unwinding the stack hurts.
  • Exceptions should be used for just that, exceptions. An exceptional
    condition is perhaps trying to allocate memory for a new object but it
    just doesn’t happen. That’s definitely exceptional. :slight_smile:
  • Use checks, of a return value or ‘global’ (ack) indicator, when
    possible or prudent. I prefer to see a check for success or failure
    than to rely on too many exceptions. Part of that can be due to my old
    COM programming days on Windows (don’t tell anyone I admitted to that).
  • Catch the exception/error as soon as possible. React/log the gory
    details and then push up if necessary with more generalized
    information.
  • Relying on exceptions too much can get ugly to read. Personally I
    find it hard to read code that uses exception catches all over the
    place. But then again, that’s just me.

I could see going both ways in your example. The <mark_as_shipped>
method sets an expected change of state to the object. Simple enough,
but if it doesn’t work the caller may wish to know that’s the case -
throw an exception. However, if this isn’t THAT exceptional perhaps
you could consider a return value of the state. The caller can check
to see if the state was indeed changed to what was expected. Maybe
even just use true/false return value for success or failure. Its
possible some callers (perhaps a batch process) may not care. if so,
why force a call to wrap it in a rescue block?

So, while I didn’t exactly answer the question I hope that gave you
some thoughts from someone who doesn’t claim to know much about the
subject at all. :slight_smile:

3

Ouch, this has always been a touch subject for me as well. I’ve never
quite felt comfortable with a philosophy on when to use Exceptions and
when to not use them. Perhaps its just one of the many brain blocks I
have been blessed with.

You’re not alone :slight_smile:

Perhaps these 5 points may be of some help:

  • Exceptions are expensive to use. They can, in some cases materially,
    affect the performance of your applicaiton. Unwinding the stack hurts.

I don’t think this is really a problem for this particular app, as
this function is limited to the backend and there won’t be much going
on at first.

  • Exceptions should be used for just that, exceptions. An exceptional
    condition is perhaps trying to allocate memory for a new object but it
    just doesn’t happen. That’s definitely exceptional. :slight_smile:

Yeah, I see where your coming from. I suppose that’s what I was really
asking. The thing is that the button or whatever that activates this
action won’t be visible if the order has been cancelled. So, it
someone tries to ship it, is that exceptional? I’m on the fence
really.

  • Use checks, of a return value or ‘global’ (ack) indicator, when
    possible or prudent. I prefer to see a check for success or failure
    than to rely on too many exceptions. Part of that can be due to my old
    COM programming days on Windows (don’t tell anyone I admitted to that).
  • Catch the exception/error as soon as possible. React/log the gory
    details and then push up if necessary with more generalized
    information.

If I simply return true or false, how can I see what caused the record
update to fail. For instance, I may want to extend my example to the
below. Tell me if this is crazy :wink:

class Order << AR:B
def mark_as_shipped
if self.status == ‘cancelled’
raise OrderCancelledException
end
if self.status == ‘unpaid’
raise OrderUnpaidException
end
# do shipping stuff
save
end
end

When I call the controller method and the update fails, if I just use
a return value I won’t know from the controller, why it failed.

  • Relying on exceptions too much can get ugly to read. Personally I
    find it hard to read code that uses exception catches all over the
    place. But then again, that’s just me.

I don’t think I’ve written or read enough code to call that yet :slight_smile:

I could see going both ways in your example. The <mark_as_shipped>
method sets an expected change of state to the object. Simple enough,
but if it doesn’t work the caller may wish to know that’s the case -
throw an exception. However, if this isn’t THAT exceptional perhaps
you could consider a return value of the state. The caller can check
to see if the state was indeed changed to what was expected. Maybe
even just use true/false return value for success or failure. Its
possible some callers (perhaps a batch process) may not care. if so,
why force a call to wrap it in a rescue block?

If someone calls the mark_as_shipped without the order being ready to
ship, that should be exceptional as they shouldn’t have the ability to
do that through the interface. I think.

So, while I didn’t exactly answer the question I hope that gave you
some thoughts from someone who doesn’t claim to know much about the
subject at all. :slight_smile:

Oh yes, thanks for your input :slight_smile:

4

Just found this thread which is making me lean towards doing this with
an exception.

http://thread.gmane.org/gmane.comp.lang.ruby.rails/80403/

5

That thread suggests using exceptions as some sort of defensive
programming paradigm, by basically claiming that if you ignore an error
returned the resulting program may be harder to debug than if you
ignore an error that is raised. It also suggests that you may be able
to carry around more error context by raising an error than by just
returning a value.

While on this subject, we should note that rails documentation suggests
that one must raise an error in order to rollback a transaction.

In C, we can’t of course raise exceptions very easily. We can treat
SIGSEGV and other types of signals along those lines as exceptions.
That philosophy says: sure, go ahead and raise an exception if you
really must, but it really is a very rare exceptional condition where
your program probably contains a bug and is having a very hard time
limping further along.

We note that if routines are willing to sometimes return a flag and
sometimes raise an exception, then we will pretty much need to wrap
every routine in logic to catch both a raised exception and to handle a
return code.

We also note that if we are going to not raise an exception, then we
need to build our own error handling paradigm. Typically we would pass
either a structure or a buffer down through our call stack so that when
an error occurs, it can record the FILE and LINE where the
error occured and save any parameters that may be of interest. Higher
layers on the stack may save additional data. And this would get
logged at an error or warning or info level.

Also, with errors, we kind of want a nice succinct well defined error
code to be generated so that when we are close to the HTML we can
decide what language we want to render the error in.

So, rails is supposed to be handing us all the important paradigms on a
platter. But it isn’t giving us a canned packaged error handling
paradigm. And it isn’t making the language translation issues any
easier.