Validation method vs. After Save & Rollback

Hey Guys,

Again, I am just learning Rails, so I am asking advice here.

I have a model called Company which has many Owners. The Owners model
indicates the name of the owner and their share (as a percentage) of
the company. The total shares for any one company can not be more than
100%.

So if I have 10 owners each with 10% of the company and I update one
of the owners and set their share to 11%, I should be given a warning
message.

This is what I have done in my Owner model:

def more_than_100_given_away?
  # => find the total share
  company = Company.find(company_id)
  company_total_share = company.total_share # => This is summing the
old value
  if changed?
    if changes["share"] # => Changes to a model are stored in the
changes array on the instance, the array has 2 value ["from_value",
"to_value"]
      old_value = changes["share"][0].to_f # => Convert To Float
      new_value = changes["share"][1].to_f
      new_total_share = company_total_share - old_value + new_value
      new_total_share > 100.01
    else
      false # validation succeeds
    end
  else
    false # validation succeeds
  end
end

It is working for me just fine, but I wanted to know if there was a
better (less NOOB looking way to do this)

Thanks!

For the most the above seems reasonable. There are a couple of things I'd watch out for though.

- Near as i can tell a company could give away 100.009 shares. Sure, .009 isn't that much, but it could be. And it's more than 100. Of course dealing with float rounding sucks. Don't know if it's an option, but you may want to consider storing the total number of shares for a company and track the number of shares per owner. That keeps everything in integer values. If you want to get an owner's percentage of ownership do the calculation then, but only for display purposes.

- The other problem is concurrency. What if I update two records at the same time and set them both to own 60% (assume individually they would succeed). They'll both succeeed if I time it right. So you should wrap this in a transaction so that can't happen.

-philip

Thanks Philip, would you mind clarifying the wrapping in a
transaction? Im just not quite sure what the means/entails

Thanks Philip, would you mind clarifying the wrapping in a
transaction? Im just not quite sure what the means/entails

You want to ensure that that piece of code can only run at once at a
time. Transaction isn't probably the best word for it. Synchronized
lock, mutex, etc. Google around for those terms.