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.