Improving on a working solution.

While we were working on some optimizaitons, we came across code that appeared to be duplicated in one of our view models.  In the view model we were setting up instance variables for our view, based on gathering data from a model.  We needed to pull data together for different products and rounds.  My task was to do some re-factoring and try to dry things up.  

While I completed my task, and the code worked fine, something was discovered when I was reviewing my change with my pair and lead on the project. Below is a modified and reduced version of the code - enough to illustrate the point:

#Example: original change in model file
def self.products_for_round(products, round)
  excess_demand, no_excess, excess_supply = [0, 0, 0]

  *** omitted ***

    if demand > product.supply
      excess_demand += 1
    elsif demand == product.supply
      no_excess += 1
    else
      excess_supply += 1
    end
  end
  [excess_demand, no_excess, excess_supply]
end

Let's take a quick minute to review.  First we are setting three variables to a value of zero (0.)  We then go through an if-else block and modify those three variables, where applicable, increasing by 1.  Lastly, we return a three element array of those variables.  Keep this in mind, because we will come back to this when we look at the view model example.

The code above worked fine.  However, the way I setup my three variables, which I thought was neat and clever, turned out to be not the most performant way.  Luckily I have a smarter and more experienced pair to point things out to me.  So after a little dicussion, the change below was made:

#Modified, more performant version of model file 
def self.products_for_round(products, round)
  excess_demand = no_excess = excess_supply = 0

  *** omitted ***

    if demand > product.supply
      excess_demand += 1
    elsif demand == product.supply
      no_excess += 1
    else
      excess_supply += 1
    end
  end
  [excess_demand, no_excess, excess_supply]
end

Why the change?  The original code used a three element array of zeros.  Then we assigned each element of the array to the respective variables, excess_demand, no_excess and excess_supply.  It works, but it is more costly from a performance point of view versus using the solution of single line multiple assignment.

After the first code example I mentioned that we should pay attention to the three element array return value from the model method.  Below is an example of code that one would find in a view model.  We set three instance variables to the result of what is returned from ProductStats.products_for_round.  Take a look: 

#Example from our view model (view)
def cur_rnd_prods
  @cur_rnd_prods_excess_demand, @cur_rnd_prods_no_excess,
  @cur_rnd_prods_excess_supply = ProductStats.products_for_round(current_round.products, current_round)
end

So in this case, it was perfectly ok to use my original thought of  setting my variables using something like:  variable_x, variable_y, variable_z = result.

Learn Something New Everyday

Last Edited by: John on February 11, 2016