Incorrect use of exception handling

Exception handling was never something that I looked much into. I've caught exceptions from libraries before, but when it came to coming up with exceptions to throw, I never gave it too much thought. So I looked it up in detail, and to my horror, I had been using it wrong.

Documenting my stupidity, hopefully, I'll prevent others from doing the same basic mistake.

In a rails controller, there is always a simple case of creating a model, but sometimes, there's value checking.

  def create
    if params[email] == "bob@uiuc.edu"
      flash[:error] = "Bobs at UIUC not allowed"
      redirect_to :action => :list
      return
    end
    @friend = Friend.create(params)
    unless @friend.save
      render :action => :edit
    else
      flash[:notice] = 'Friend was successfully updated'
      redirect_to :action => 'list'
    end
  end

I thought, "Hey, why not move that error handling code to the end, so it reads better?"

  class NoBobsError < Exception; end
  def create
    raise NoBobsError.new if params[email] == "bob@uiuc.edu"

    @friend = Friend.create(params)
    raise ActiveRecord::RecordNotSaved.new unless @friend.save

    flash[:notice] = 'Friend was successfully updated'
    redirect_to :action => 'list'
  rescue ActiveRecord::RecordNotSaved
    render :action => :edit    
  rescue Exception
    flash[:error] = "Bobs at UIUC not allowed"
    redirect_to :action => :list
  end

That way, the error handling code doesn't really get in the way of the 'good condition' code. I personally think it's easier to read, though apparently, this is a bad idea apparently, mostly due to overhead costs in running through an exception, even if there were no exceptions thrown, and that I'm essentially using it as a goto statement. And as everyone knows, gotos taste like ass.

Exceptions are to be used when the method or object that the error occurred doesn't know what to do with the error at that time. Therefore, it will throw an exception, and hope that some other part of the code elsewhere up the stack will know what to do with it. And so hence the adage: "throw early, catch late".

So I'm a reformed exception handling abuser. I guess when you have a new hammer, the world looks like a nail, until someone sets you right.


http://www.onjava.com/pub/a/onjava/2003/11/19/exceptions.html
http://today.java.net/pub/a/today/2003/12/04/exceptions.html