checking user model variable using nested if statements

Question

I'm trying to perform a check on my User model using a series of nested if statements. I have two models, User, which has_many :taxes, and tax which belongs_to :user. Within a column of the user table, each user has a specific "state id" which they select when they sign up (for example, state_id: 53 could mean California). Anyway, in my Tax model, I have several if statements which evaluate to see if a user is from a specific region, and if true, the conditions inside the if are performed.

For example:

if user_state_id == 53
   # Do this
end

The problem is, if the user isn't a match to the first if statement, it doesn't continue to check the remaining statements to see if it matches any of the others. (See Below). If this user had a state_id of 53, it would work normally and run the remaining conditions inside. However, if the user had a state_id of 52 (next in the list) it would not evaluate.

def provincial
  if user.state_id == 53 #BC
    if calculation = self.income <= 10276
      return 0
    elsif calculation = self.income > 10276 && self.income <= 37568
      return self.income * 0.0506
    end
  end
  if user.state_id == 52 #AB
    if calculation = self.income <= 17593
      return 0
    elsif calculation = self.income < 17593
      return self.income * 0.10
    end
  end
  if user.state_id == 60 #ON
    if calculation = self.income <= 9574
      return 0
    elsif calculation = self.income > 9574 && self.income <= 39723
      return self.income * 0.0505
    end
  end
end

I also tried using elsif statements for the primary nested condition checks (elsif user.state_id == 52...) but that did not work either.

Not really a solution, but this kind of multiple choice logic is usually implemented using [the case statement](http://stackoverflow.com/questions/948135/how-can-i-write-a-switch-statement-in-ruby). — Leonid Shevtsov, Feb 04 '14 at 09:22
How can a user have two different `state_id`s at the same time? Are they quantum users? =) — mdesantis, Feb 04 '14 at 09:22
What is you calculation? You are assigning it and never use it. Also it probably should go to http://codereview.stackexchange.com/ — BroiSatse, Feb 04 '14 at 09:24
@mdesantis an individual user doesn't have more than one state_id, but there are many users which can be from different states, so it's to account for them. — Jon Girard, Feb 04 '14 at 09:50

BroiSatse · Answer 1 · 2014-02-04T09:35:24.103

0

In your nested if statement for state_id == 52 you are checking twice whether income is smaller then 17593 (in both if and elseif).

if user.state_id == 52 #AB
  if calculation = self.income <= 17593
    return 0
  elsif calculation = self.income < 17593 # shouldn't it be > 17593?
    return self.income * 0.10
  end
end

Also you are not covering all the cases! If state_id is equal to 53, and income is bigger than 37568, this will return nil.

Another thing, if you checked in you if statement that self.income <= 10276 (self is not needed btw), you don't need to recheck it again in elsesif (elseif self.income > 10276). There is no other option.

Your calculation variable is completely obsolete - your function returns if it is true and continues when its false.

edited Feb 04 '14 at 09:35

answered Feb 04 '14 at 09:28

BroiSatse

44,031
8
61
86

Thanks, I didn't post the full code in which I cover many more cases. So is there a better way of doing something like this, since I've been told it looks terrible. Maybe with a case statement? – Jon Girard Feb 04 '14 at 09:51
Case statement is definitively a good way to go, howvere things like code style and refactoring should be done on codereview stackexchange. :) – BroiSatse Feb 04 '14 at 09:58

Pavan · Accepted Answer · 2014-02-04T10:53:28.670

0

I just posting a review of your code by using nested case statements.Sorry for the syntax errors(if any)

case user.state_id

when 53     #BC

      case calculation
       when <=10276
       return 0
       when 10277..37568
       return self.income * 0.0506

when 52     #AB
       case calculation
       when 17593
       return 0
        when < 17593
        return self.income * 0.10

when 60     #ON
         case calculation
         when <= 9754
         return 0
         when 9575..39793
         return self.income * 0.0505

Hope it helps!

edited Feb 04 '14 at 10:53

answered Feb 04 '14 at 10:35

Pavan

33,316
7
50
76

this more or less ended up being the correct answer, except with the < symbols in the when statement, a proc needs to be used (proc {|n| n > 17593}) otherwise it will throw a syntax error – Jon Girard Feb 04 '14 at 21:28

checking user model variable using nested if statements

2 Answers2