first attempt: array of hashes, please help

Hello, I've been working on this project quite a bit. I'm very enthusiastic about learning Ruby, and have done my best to comment with MARKERs where the problems are or may be. The hurdle is that I need output to print values for each listed unit of measure per food item, in this case salted butter. The units are cup, tbsp, pat, and stick. I can only seem to get the printouts for cup or stick. Please review my code and help me fix it, or suggest more efficient alternatives. Thank you kindly.

def nutrients(food_id)     @food = FoodDescription.find(food_id)     @nutrients =     @nutrients = NutrientDatum.find_all_by_ndb_id(@food.ndb_id)

    @n = 0     @definition =     @values =

      @nutrient_id = @nutrients[@n].nutrient_id       @nutrient_description = NutrientDefinition.find_by_nutrient_id(@nutrient_id).description       @definition[@n] = @nutrient_description       # TODO calculate for units serving

      @values[@n] = @nutrients[@n].nutrient_value

      @units = units(@food.ndb_id)       @m = 1       @composite =       until @m > @units.length         @composite[@m - 1] = calculate(@food, @nutrients, @m)         @m += 1       end

    # MARKER 1 # print @out returns @composite[0 through 8]                # but only for the last unit, i.e. cup(1),                # tbsp(2), pat(3), => stick(4) of butter...                # I don't know why it's not printing 4 arrays,                # eight hashes in each array

    @out = @composite     #system("echo #{@out}")     print @out   end

  def units(ndb_id)     @units =     @units = FoodWeight.find_all_by_ndb_id(ndb_id)     @units   end

  def calculate(food, nutes, unit)

      @n = 0       calories = {} # MARKER 2 # may be a problem here       energy = {}       protein = {}       fats = {}       carbs = {}       vitamins = {}       minerals = {}       sterols = {}       other = {}       @composite =     until @n == nutes.length

      info = FoodWeight.find_by_ndb_id_and_sequence_id(food.ndb_id, unit)       results = {}       results[:amount] = info.amount == 1.0 ?                        results[:amount] = 1 : results[:amount] = info.amount       results[:description] = info.unit_description       results[:grams] = info.gram_weight     # TODO get nutrient weights per unit weight

      @description = NutrientDefinition.find(@n + 1).description       @grams_unit = (nutes[@n].nutrient_value / 100 * results[:grams]).to_i       @key = " #{results[:amount]} #{results[:description]} #{@grams_unit} \n"

      case @description         # Main Totals         when "Protein" then protein[:total_protein] = @key         when "Total lipid (fat)" then fats[:total_fat] = @key         when "Carbohydrate, by difference" then carbs[:total_carbs] = @key         when "Energy" then           if @description == "kcal"             calories[:energy_kcal] = @key           else             calories[:energy_kj] = @key           end

        # Carbs         when "Starch" then carbs[:starch] = @key         .         .         .         # Minerals         when "Calcium, Ca" then minerals[:calcium] = @key         .         .         .         # Vitamins         when "Manganese, Mn" then vitamins[:manganese] = @key         .         .         .         # Protein and amino acids         .         .         .         # Fats         .         .         .         # Sterols         .         .         .         # Other         .         .         .         when "Theobromine" then other[:theobromine] = @key       end       @n += 1     end       # TODO calculate energy

          @composite[0] = calories           @composite[1] = energy           @composite[2] = protein           @composite[3] = fats           @composite[4] = carbs           @composite[5] = vitamins           @composite[6] = minerals           @composite[7] = sterols           @composite[8] = other

    @composite   end

== Sample output ... sodium 1 stick 772 calcium 1 stick 3 zinc 1 stick 178 iron 1 stick 0 copper 1 stick 0 magnesium 1 stick 1 fluoride 1 stick 2 phosphorus 1 stick 2823 cholesterol 1 stick 11 phytosterols 1 stick 0 caffeine 1 stick 2 theobromine 1 stick 27 ...

2 problems, 1 big, one not so significant

An @ sign declares an instance variable, that is, it persists through the entire class. You have @composite as a class variable, so every time you set it to something, it erases what you had it at last. You have every variable declared as an instance variable when you would most likely be fine if none of them were.

Here’s an instance variable example: you have a person object with a first_name and last_name attribute. You would use first_name and last_name as instance variables because they tell you something about the state of the particular instance of the object. You could have a method called full_name that returns @first_name + " " + @last_name.

The second thing I noticed was

until @m > @units.length

The more elegant ruby way to do this is:

@units.length.each do |val| composite[val-1] = … end

I suggest you pick up a copy of Programming Ruby by the pragmatic programmers. I don’t know how I would have gotten by without it when I was learning.

Hope this helps

Sorry, I made a mistake:

An @ sign declares an instance variable, that is, it persists through the entire class.

An instance variable persists through the entire INSTANCE of a class, a class variable (denoted by @@) persists through all instances of a class.

WOW! I was tinkering for hours, and made only this change, and presto!

      composite =       until @m > @units.length         composite[@m - 1] = calculate(@food, @nutrients, @m)         @m += 1       end

I've got access to values for each unit per food item in the whole database.

Thanks !!

Joe K wrote:

You’re very welcome. I strongly urge you though to get your variable types straightened out before things get out of hand. A little labor in getting all these fixed now will prevent tons of problems in the future. All these instance variables may also have an impact on performance as well since they persist in memory for the life of the instance (it wouldn’t be something that would apparent until you are getting lots of concurrent hits). A perfect example is below. You have a counter there (@m) that persists for the entire life of the instance when ideally, you want to destroy it when the loop is finished. Read up on ruby variable types and scope.

WOW! I was tinkering for hours, and made only this change, and presto!

     composite =      until @m > @units.length        composite[@m - 1] = calculate(@food, @nutrients, @m)        @m += 1      end

I've got access to values for each unit per food item in the whole database.

Thanks !!

Joe K wrote:

The second thing I noticed was

until @m > @units.length

The more elegant ruby way to do this is:

@units.length.each do |val|    composite[val-1] = ... end

On Fri, Aug 22, 2008 at 1:13 PM, Jesse Crockett <

At first, I was just going to comment on this:

Since @m seems to be a value from 1 to @units.length, the better way would be something like:

(1..@units.length).each do |m|    composite[m-1] = calculate(@food, @nutrients, m) end

But then I had to go back to your original post to see why you weren't iterating over @units rather than the index.

So here's a longer commentary:

Hello, I've been working on this project quite a bit. I'm very enthusiastic about learning Ruby, and have done my best to comment with MARKERs where the problems are or may be. The hurdle is that I need output to print values for each listed unit of measure per food item, in this case salted butter. The units are cup, tbsp, pat, and stick. I can only seem to get the printouts for cup or stick. Please review my code and help me fix it, or suggest more efficient alternatives. Thank you kindly.

def nutrients(food_id)    @food = FoodDescription.find(food_id)    @nutrients =

No need to initialize this as the next line already does that.

   @nutrients = NutrientDatum.find_all_by_ndb_id(@food.ndb_id)

Perhaps have some ActiveRecord associations (note that's I'm guessing at some of these):

class FoodDescription    belongs_to :ndb    delegates :nutrient_data, :to => :ndb end class Ndb    has_many :nutrient_data end class NutrientDatum    belongs_to :ndb end

Then you can say:

   @nutrients = @food.nutrient_data

   @n = 0    @definition =    @values =

     @nutrient_id = @nutrients[@n].nutrient_id      @nutrient_description = NutrientDefinition.find_by_nutrient_id(@nutrient_id).description

It looks like you could also define the relation: class NutrientDatum    belongs_to :nutrient    # attributes: nutrient_value end class Nutrient    has_one :nutrient_definition    delegates :description, :to => :nutrient_definition end class NutrientDefinition    belongs_to :nutrient    # attributes: description end

     @definition[@n] = @nutrient_description      # TODO calculate for units serving

     @values[@n] = @nutrients[@n].nutrient_value

     @units = units(@food.ndb_id)

Yikes! Like Joe K said, you need to get your variable types straightened out. The units() method already has access to @food, it creates/assigns @units, and then you assign that object to @units again here!

     @m = 1      @composite =      until @m > @units.length        @composite[@m - 1] = calculate(@food, @nutrients, @m)        @m += 1      end

   # MARKER 1 # print @out returns @composite[0 through 8]               # but only for the last unit, i.e. cup(1),               # tbsp(2), pat(3), => stick(4) of butter...               # I don't know why it's not printing 4 arrays,               # eight hashes in each array

   @out = @composite    #system("echo #{@out}")    print @out end

def units(ndb_id)    @units =    @units = FoodWeight.find_all_by_ndb_id(ndb_id)    @units end

Another association: class FoodWeight    belongs_to :nutrient_datum, :foreign_key => 'ndb_id' end class NutrientDatum    has_many :food_weights end

class FoodDescription    delegates :food_weights, :to => :ndb end

Then you can just refer to:    def units      @food.food_weights    end

def calculate(food, nutes, unit)

     @n = 0      calories = {} # MARKER 2 # may be a problem here      energy = {}      protein = {}      fats = {}      carbs = {}      vitamins = {}      minerals = {}      sterols = {}      other = {}      @composite =    until @n == nutes.length

     info = FoodWeight.find_by_ndb_id_and_sequence_id(food.ndb_id, unit)      results = {}      results[:amount] = info.amount == 1.0 ?                       results[:amount] = 1 : results[:amount] = info.amount

At the very least, this ought to be:        results[:amount] = info.amount == 1.0 ? 1 : info.amount But the chance that info.amount is exactly 1.0 is low (floating point comparisons being notoriously prone to small errors and 1.0 will behave like 1 anywhere it's likely to matter in an application such as this).

So it's just:        results[:amount] = info.amount

     results[:description] = info.unit_description      results[:grams] = info.gram_weight

If you just want to initialize results, you could more compactly say:

       results = { :amount => info.amount,                    :description => info.unit_description,                    :grams => info.gram_weight }

   # TODO get nutrient weights per unit weight

     @description = NutrientDefinition.find(@n + 1).description      @grams_unit = (nutes[@n].nutrient_value / 100 * results[:grams]).to_i      @key = " #{results[:amount]} #{results[:description]} #{@grams_unit} \n"

     case @description        # Main Totals        when "Protein" then protein[:total_protein] = @key        when "Total lipid (fat)" then fats[:total_fat] = @key        when "Carbohydrate, by difference" then carbs[:total_carbs] = @key        when "Energy" then          if @description == "kcal"            calories[:energy_kcal] = @key          else            calories[:energy_kj] = @key          end

       # Carbs        when "Starch" then carbs[:starch] = @key        .        # Minerals        when "Calcium, Ca" then minerals[:calcium] = @key        .        # Vitamins        when "Manganese, Mn" then vitamins[:manganese] = @key        .        # Protein and amino acids        .        # Fats        .        # Sterols        .        # Other        .        when "Theobromine" then other[:theobromine] = @key      end

I'd probably back this all with a model (that might not be in the database) that would know the nutrient classification. You also seem to avoid using Strings as hash keys. You could have something like:

     case classification_for(@description)      when :protein        protein[@description] = @key      when :fats        fats[@description] = @key      when :carbs        carbs[@description] = @key

... WAIT! That seems awfully repetitious. If composite weren't an array, but a hash, then you could have:

     composite[classification_for(@description)][@description] = @key

Assuming an implementation such as: def classification_for(description)    case description    when "Protein"      :protein    when "Total lipid (fat)"      :fats    when "Carbohydrate, by difference", 'Starch'      :carbs   ... end

Can you start to see why this would be good to add as an attribute of the NutrientDefinition?

     @n += 1    end      # TODO calculate energy

         @composite[0] = calories          @composite[1] = energy          @composite[2] = protein          @composite[3] = fats          @composite[4] = carbs          @composite[5] = vitamins          @composite[6] = minerals          @composite[7] = sterols          @composite[8] = other

   @composite end

If you want to produce the output in a particular order, you can always iterate over an ordered array of keys.

[ :calories, :energy, :protein, :fats, ...].each do |kind|    puts kind    composite[kind].each do |description,text|      puts "#{description} #{text}"      # or something like:      # puts "%-30s %s"%[description, text]    end end

== Sample output ... sodium 1 stick 772 calcium 1 stick 3 zinc 1 stick 178 iron 1 stick 0 copper 1 stick 0 magnesium 1 stick 1 fluoride 1 stick 2 phosphorus 1 stick 2823 cholesterol 1 stick 11 phytosterols 1 stick 0 caffeine 1 stick 2 theobromine 1 stick 27 ...

In addition to thinking (learning) about variable types, you should also look into the complexity that ActiveRecord associations can cover for you. (Note that were I've reopened the model classes several times, you'd generally have all the associations in one place in each model.)

-Rob

Rob Biedenharn http://agileconsultingllc.com Rob@AgileConsultingLLC.com

Hrm, you mean like Thursday when pulling 500k records took 3.5 hours?

Also, I've changed the large case statement to check against the nutrient ids instead of their string descriptions. Much more accurate.

I started learning rails with RailsSpace, and I was perplexed over the usage of 'object' instead of '@object'.

Is there a quick rule of thumb when instance variables are preferred over their non-delimited counterpart?

You should only use an instance variable when it holds some part of the state of the object. Most normal computation and data manipulation should use simple local variables. It's also very useful to obtain the data from the object that "owns" it when it is needed rather than carrying it around in a local variable outside the object.

In many applications, the total amount of data far exceeds the amount of code that manipulates it. This is one reason that focussing on a good data structure first will often yield great dividends later.

-Rob

Joe K wrote:

You're very welcome. I strongly urge you though to get your variable types straightened out before things get out of hand. A little labor in getting all these fixed now will prevent tons of problems in the future. All these instance variables may also have an impact on performance as well since they persist in memory for the life of the instance (it wouldn't be something that would apparent until you are getting lots of concurrent hits). A perfect example is below. You have a counter there (@m) that persists for the entire life of the instance when ideally, you want to destroy it when the loop is finished. Read up on ruby variable types and scope.

Rob Biedenharn http://agileconsultingllc.com Rob@AgileConsultingLLC.com

Thank you :slight_smile: I've just started reading the PickAxe, alongside The Rails Way. Also the weekly paycheck will be good incentive to get up to speed and stay on the ball. thanks again.