http://ngauthier.com/2011/09/using-exceptions-to-manage-control-flow.html Your Thoughts?

ngauthier_reverse_auth_controller.rb

# This controller's job is to exchange twitter credentials for Shortmail credentials
class TwitterReverseAuthController < ApplicationController
  # First, let's make our own subclass of RuntimeError
  class Error < RuntimeError; end

  def api_key_exchange
    # Here are our required parameters. If any are missing we raise an error
    screen_name = params.fetch(:screen_name) { raise Error.new('screen_name required')  }
    token =       params.fetch(:oauth_token) { raise Error.new('oauth_token required')  }
    secret =      params.fetch(:oauth_secret){ raise Error.new('oauth_secret required') }

    # OK now let's authenticate that user. If we can't find a valid user, raise an error
    @user = User.by_screen_name(screen_name).where(
      :oauth_token => token,
      :oauth_secret => secret
    ).first or raise Error.new('user not found')

    # Now we'll build a device. I'm not catching an exception on create! here because
    # It should never fail. (I.e. a failure is actually a 500 because we don't expect it)
    @device = Device.find_or_create_by_token!(
      params.slice(:token, :description).merge(:user_id => @user.id)
    )

    render :json => { :api_key => @device.api_key }

  # Now I can simply catch any of my custom exceptions here
  rescue Error => e
    # And render their message back to the user
    render :json => { :error => e.message }, :status => :unprocessable_entity
  end
end

twitter_reverse_auth_controller.rb

# This controller's job is to exchange twitter credentials for Shortmail credentials
class TwitterReverseAuthController < ApplicationController

  def api_key_exchange
    auth = OauthAuthenticator.new(params)
    if auth.is_valid?
      render :json => { :api_key => auth.response[:api_key] }
    else
      render :json => {
        :error => auth.response[:errors].full_messages.join(", "),
        :status => :unprocessable_entity
      }
    end
  end


  class OauthAuthenticate < OpenStruct
    #magic_attrs: screen_name,oauth_token,oauth_secret,token,description
    #public
    def is_valid?
      self.validate_device
      self.valid?
    end
    def response
      {:errors => self.errors, :api_key =>self.device && self.device.api_key}
    end

    #private
    
    include ActiveModel::Validations; def to_model; self; end

    def find_user
      User.by_screen_name(screen_name).
                   where(:oauth_token => oauth_token,
                         :oauth_secret => oauth_secret).first
    end
    def find_device
      Device.find_or_create_by_token({
                                       :token => token,
                                       :description => description,
                                       :user_id => user.id
                                     })
    end
    
    validates_presence_of :screen_name, :oauth_token, :oauth_secret, :message => "required"
    def validate_user
      return self unless valid?
      self.user = find_user
      self.errors.add(:user, 'not found') unless user
      return self
    end
    def validate_device
      return self unless validate_user && valid?
      self.device = find_device
      unless device
        self.errors.add(:device, 'not found') 
        device.errors.each do |k,m|
          self.errors.add(k,m)
        end unless device.saved?
      end
      return self
    end
  end
end

Basically, I think he unnecessarily gives up too much information this way, and if he's really concerned with getting non-response logic out of his controller, the best way is to move it somewhere else.

Here's what I would have preferred to see, in important-first order:

Things I'll hold constant:

• When passed incorrect screen_name, oauth_token, or oauth_secret, it will return a json requirement message
• When it cannot authenticate the user, it will return a json not foundmessage
• When its able to correctly authenticate, it will return a json api_key
• It does no additional validations of the description and 'token' beyond what Device implements in a hidden fashion via active record
• I'll add only one class

Things I'll change in the api:

• When the device has issues, it will return json rather than looking for a html view to render the exception and it will return a status code in the 400-499 range
• When more than one error occurs, it will list each that has occurred (smartly.)

Things I'll change in the mechanism:

• Add a class that understands the error conditions
• Remove useless controller instance variables
• The controller will only know the http/json interface.