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

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

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:

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

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

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.

On Aug 22, 2008, at 2:27 PM, Jesse C. wrote:

database.

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

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

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:

([email protected]).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:

On Aug 22, 2008, at 1:13 PM, Jesse C. wrote:

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!

          #  tbsp(2), pat(3), => stick(4) of butter...

@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

 carbs    = {}
 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 }
 case @description
     end
   .
   .
   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?

     @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

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 B. http://agileconsultingllc.com
[email protected]

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?

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.

On Aug 23, 2008, at 8:28 AM, Jesse C. wrote:

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

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 B. http://agileconsultingllc.com
[email protected]

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.