views:

47

answers:

3

I'm trying to recalculate percentages in an after_update callback of my model.

  def update_percentages
    if self.likes_changed? or self.dislikes_changed?
      total = self.likes + self.dislikes

      self.likes_percent = (self.likes / total) * 100
      self.dislikes_percent = (self.dislikes / total) * 100
      self.save
    end
  end

This doesn't work. The percentage always comes out as a 100 or 0, which completely wrecks everything.

Where am I slipping up? I guarantee that self.likes and self.dislikes are being incremented correctly.

+1  A: 

I'm not sure that this is the only problem that you have, but after_update gets called after the object is saved.

Try changing the update_percentages before - on a before_update or a before_validate instead. Also, remove the self.save line - it will be called automatically later on if you use one of those callbacks.

egarcia
Thanks, you're right, that is better.
Matt H
+2  A: 

Because likes/dislikes is an integer value and integer/integer = integer.

so you can do one of two things, convert to Float or change your order of operations.

self.likes_percent = (self.likes.to_f/total.to_f) * 100

Or, to keep everything integers

self.likes_percent = (self.likes * 100)/total
Geoff Lanotte
+4  A: 

The Problem

When you divide an integer by an integer (aka integer division), most programming languages, including Ruby, assume you want your result to be an Integer. This is mostly due to History, because with lower level representations of numbers, an integer is very different than a number with a decimal point, and division with integers is much faster. So your percentage, a number between 0 and 1, has its decimal truncated, and so becomes either 0 or 1. When multiplied by 100, becomes either 0 or 100.

A General Solution

If any of the numbers in the division are not integers, then integer division will not be performed. The alternative is a number with a decimal point. There are several types of numbers like this, but typically they are referred to as floating point numbers, and in Ruby, the most typical floating point number is of the class Float.

1.0.class.ancestors
  # => [Float, Precision, Numeric, Comparable, Object, Kernel]

1.class.ancestors
  # => [Fixnum, Integer, Precision, Numeric, Comparable, Object, Kernel]

In Rails' models, floats are represented with the Ruby Float class, and decimal with the Ruby BigDecimal class. The difference is that BigDecimals are much more accurate (ie can be used for money).

Typically, you can "typecaste" your number to a float, which means that you will not be doing integer division any more. Then, you can convert it back to an integer after your calculations if necessary.

x = 20              # => 20
y = 30              # => 30
y.to_f              # => 30.0

x.class             # => Fixnum
y.class             # => Fixnum
y.to_f.class        # => Float

20 / 30             # => 0
20 / 30.0           # => 0.666666666666667

x / y               # => 0
x / y.to_f          # => 0.666666666666667

(x / y.to_f).round  # => 1

A Solution For You

In your case, assuming you are wanting integer results (ie 42 for 42%) I think the easiest way to do this would be to multiply by 100 before you divide. That pushes your decimal point as far out to the right as it will ever go, before the division, which means that your number is as accurate as it will ever get.

before_save :update_percentages
def update_percentages
  total = likes + dislikes
  self.likes_percent     =  100 * likes / total
  self.dislikes_percent  =  100 * dislikes / total
end

Notes:

  • I removed implicit self you only need them on assignment to disambiguate from creating a local variable, and when you have a local variable to disambiguate that you wish to invoke the method rather than reference the variable
  • As suggested by egarcia, I moved it to a callback that happens before the save (I selected before_save because I don't know why you would need to calculate this percentage on an update but not a create, and I feel like it should happen after you validate that the numbers are correct -- ie within range, and integers or decimal or whatever)
  • Because it is done before saving, we remove the call to save in the code, that is already going to happen
  • Because we are not explicitly saving in the callback, we do not risk an infinite loop, and thus do not need to check if the numbers have been updated. We just calculate the percentages every time we save.
Joshua Cheek