Topic: refactor a Case statement

is there a way of improving this code?

  def confirm_current_user_account
    case @customer_account.customer_type
    when 'User' then
      if @customer_account.customer == current_user
        @customer = @customer_account
      else
        redirect_to bank_root_path, alert: "Ocurrio un problema (usuario)."
      end
    when 'Company' then
      unless current_user.companies.find(@customer_account.customer).nil?
        @customer = @customer_account
      else
        redirect_to bank_root_path, alert: "Ocurrio un problema (empresa)."
      end
    else
      redirect_to bank_root_path, alert: "Ocurrio un problema (general)."
    end
  end

Last edited by jtomasrl (2011-10-12 23:54:24)

Re: refactor a Case statement

I'd say not really, not directly.

Question , do you find yourself with lots of these case statements littered throughout your code?

You might look into Single Table Inheritance, STI.

It would be cleaner code if you had TWO different confirm_current_user_account methods, neither with a case statement,  STI could get you there.

check this article:

http://code.alexreisner.com/articles/si … rails.html

Joe got a job, on the day shift, at the Utility Muffin Research Kitchen, arrogantly twisting the sterile canvas snout of a fully charged icing anointment utensil.

Re: refactor a Case statement

Answer to your question, not really, its my only case statement in my whole app. i'll read that

Re: refactor a Case statement

Love your wine, drinking it as we speak!

Here is my take.

You'll probably have a whole bunch of code that applies to a logged in user, regardless of whether they are a User or Company, i.e. reset_my_password,  etc.

Then there are a bunch of functions that are specific to the role of 'User' or 'Company'.

So the best approach in that case is to define a base 'logged_in_user' class,  where functions that are common to ALL logged_in_users are defined,  then you subclass logged_in_user into 'real_user' and 'company_user'.

But you'll know best once you read that article where to turn.

Love your wine, did I mention that.....  :>

Joe got a job, on the day shift, at the Utility Muffin Research Kitchen, arrogantly twisting the sterile canvas snout of a fully charged icing anointment utensil.

Re: refactor a Case statement

In that case I would reduce some conditions and make it more linear:
if @customer_account.customer_type == 'User' && @customer_account.customer == current_user
    @customer = @customer_account
else

if @customer_account.customer_type == 'Company' && current_user.companies.find(@customer_account.customer).present?
  @customer = @customer_account
end

redirect_to bank_root_path, alert: "Ocurrio un problema (general)." if @customer.nil?

Also maybe it would be good to add variables for conditions:
user_condition = @customer_account.customer_type == 'User' && @customer_account.customer == current_user

company_condition = @customer_account.customer_type == 'Company' && current_user.companies.find(@customer_account.customer).present?

@customer = @customer_account if user_condition || company_condition
redirect_to bank_root_path, alert: "Ocurrio un problema (general)." if @customer.nil?

Re: refactor a Case statement

you can try to refactor it towards to strategy pattern.