CODEWITHSUNDEEP
ruby-on-railsrubymethods
Thanks for all the fish
Thanks for all the fish

Reputation: 1701

Rails not recognizing true or false

I have something that should be really simple, but it's killing me.

l = LineItem.first
#<LineItem id: 5, product_id: 1, quantity: 1, price: #<BigDecimal:7f7fdb51a3f8,'0.999E3',9(18)>, cart_id: 5, discount_percentage: 10, discount_amount: nil, discount_active: true, created_at: "2012-01-12 16:17:41", updated_at: "2012-01-12 16:17:41">

I have

l.discount_percentage.blank?
=> false

So, I have the following method:

  def total_price
    discount_amount = 0 if discount_amount.blank?
    discount_percentage = 0 if discount_percentage.blank?

    discounted_amount_from_percent = price*(discount_percentage.to_f/100)

    applicable_discount = [discount_amount,discounted_amount_from_percent].max

    return (price-applicable_discount)
  end

But when I do this:

l.total_price

Instead of returning 899, it returns 999 (meaning that the if discount_percentage.blank? didn't work at all!)

Or the syntax WHATEVER_HERE if true/false only work in the View on Rails??

Upvotes: 2

Views: 276

Answers (2)

tokland
tokland

Reputation: 67900

Here lays the problem:

discount_amount = 0 if discount_amount.blank?
discount_percentage = 0 if discount_percentage.blank?

Ruby "sees" variables from top to bottom and from left to right, so in that line he first sees a local variable (discount_amount =) so he decides this discount_amount thing in discount_mount.blank? is that same local variable (and not the instance method. You think the variable is not defined yet, but Ruby has already spotted it). Not having any value yet, discount_amountit set to default value nil, so nil.blank? succeeds and the assignment discount_percentage = 0 is made. Ditto for discount_percentage. Here's a demo snippet:

class ExampleClass 
  def run
    x = "it works as expected" if x == "x"
    x
  end

  def run2
    if x == "x"
      x = "it works as expected" 
    end
    x
  end

  def run3
    xy = "it works as expected" if x == "x"
    xy
  end

  def x; "x"; end
end

p ExampleClass.new.run #=> nil
p ExampleClass.new.run2 #=> "it works as expected"
p ExampleClass.new.run3 #=> "it works as expected"

Step 1: don't use the same names for local variables and instance methods. That's usually a bad idea anyway because you lose track of which one you are using, but in this case it has really bitten you.

Step 2: Do not write imperative code when you're doing math calculations! Really, maths (9X % of the things you do in a typical application, (10-X)% being unavoidable side-effects) play well with expressions, not with statements. I'd write:

def total_price
  final_discount_amount = discount_amount || 0
  final_discount_percentage = discount_percentage || 0
  discounted_amount_from_percent = price * (final_discount_percentage.to_f/100)
  applicable_discount = [final_discount_amount, discounted_amount_from_percent].max
  price - applicable_discount
end

Upvotes: 6

Behrang Saeedzadeh
Behrang Saeedzadeh

Reputation: 47951

When using attribute writers (e.g foo = ...), you should use self explicitly. This is nicely explained here.

So your code should be like this:

 def total_price
    self.discount_amount = 0 if discount_amount.blank?
    self.discount_percentage = 0 if discount_percentage.blank?

    # local var, self not necessary
    discounted_amount_from_percent = price*(discount_percentage.to_f/100)

    # local var, self not necessary
    applicable_discount = [discount_amount,discounted_amount_from_percent].max

    return (price-applicable_discount)
  end

This is also explained in the Programming Ruby book:

Why did we write self.leftChannel in the example on page 74? Well, there's a hidden gotcha with writable attributes. Normally, methods within a class can invoke other methods in the same class and its superclasses in functional form (that is, with an implicit receiver of self). However, this doesn't work with attribute writers. Ruby sees the assignment and decides that the name on the left must be a local variable, not a method call to an attribute writer.

Upvotes: 1

Related Questions