Simplify multiple if-then rake task

Question

I got the below Rake Task, that runs one a day to settle some bets in my application. But I'm pretty sure it can be simplyfied, so the IF-THEN statements can b replaced with s CASE stement.

I assume the multiple IF statements takes longer to run then the same process with the CASE option. Is that correct?

I'm just not sure how to convert it from the IF-THEN to the CASE option..

task :settle => :environment do
  @bets = Bet.where(:settle => false)

  @bets.each do |bet|

    if not bet.value.nil?

     if bet.price.value > bet.value and bet.buy == true then 
      bet.profitloss  = 10
      bet.settle      = true
      bet.save
     end

     if bet.price.value < bet.value and bet.buy == false then
      bet.profitloss  = 10
      bet.settle      = true
      bet.save
     end

     if bet.price.value > bet.value and bet.buy == false then
      bet.profitloss  = -10
      bet.settle      = true
      bet.save
     end

     if bet.price.value < bet.value and bet.buy == true then
      bet.profitloss  = -10
      bet.settle      = true
      bet.save
     end

     if bet.price.value = bet.value then
      bet.profitloss  = -10
      bet.settled     = true
      bet.save
     end 

   end
  end
end

Also, you may want to consider moving this logic into a method like `Bet.settle_all` and then just call it from here. It's easier to maintain business logic if it lives inside your models. You should also use `find_each` instead of `each` if you plan on having a large number of Bet records. And last but not least, consider moving the inside of the `each` block into an instance method `Bet#settle` and extracting some of the if statements into private instance methods. — Grant Hutchins, Sep 29 '12 at 20:22

score 3 · Answer 1 · answered Sep 29 '12 at 20:17

3

It would seem to be

bet.settle is always true bet.profitloss is -10 unless (bet.price.value > bet.value and bet.buy) or (bet.price.value < bet.Value and !bet.buy) in which case it's 10

Got this from the following truth table

< bet | = bet | > bet | buy || settle || profit
  0   |   0   |   1   |  0  ||    1   ||   0
  0   |   0   |   1   |  1  ||    1   ||   1
  0   |   1   |   0   |  0  ||    1   ||   0
  0   |   1   |   0   |  1  ||    1   ||   0
  1   |   0   |   1   |  0  ||    1   ||   1
  1   |   0   |   1   |  1  ||    1   ||   0

So you could pretty much do the entire thing with one if statement, get weaving with the tests though. :)

answered Sep 29 '12 at 20:17

Tony Hopkinson

20,172
3
31
39

sounds right.. but yes, it's seems to get really messy if it's going to be in one if statement.. Will it give any performance gain to do it in one line instead of jethroos solution? – Twiddr Sep 30 '12 at 19:20
Unless you are settling a lot of bets in any one run, I seriously doubt it. – Tony Hopkinson Oct 01 '12 at 00:00

score 2 · Accepted Answer · answered Sep 29 '12 at 20:07

maybe you can implement it like:

task :settle => :environment do
@bets = Bet.where(:settle => false)

@bets.each do |bet|
  if not bet.value.nil?
    case 
    when bet.price.value > bet.value and bet.buy == true
      profitloss  = 10
    when bet.price.value < bet.value and bet.buy == false 
      profitloss  = 10
    when bet.price.value > bet.value and bet.buy == false
      profitloss  = -10
    when bet.price.value < bet.value and bet.buy == true 
      profitloss  = -10
    when bet.price.value = bet.value
      profitloss  = -10
    end
    if profitloss  
      bet.settled = true
      bet.profitloss = profitloss 
      bet.save
    end 
  end
end

pulling out the settled and save things allows you to easily intergrate other functionality if needed without having it to copy in every if or when case.

You should pull the assignments to "profitless" outside the case. — akim, Jun 22 '21 at 13:54

Simplify multiple if-then rake task

2 Answers2