0

I am working on a ruby module that matches the first item in a 2-dimentional array (imported from a csv file), and returns the second item. It sounds extremely simple, and I was able to get it to work, until I tried to match an item that wasn't in the array. When that happens, for some reason, the entire array is returned. I was able to rig a work-around, involving a boolian variable 'found', but I would like to know why this doesn't work as written.

require 'csv'

class Nutrition

@list = CSV.read("./lib/list.csv")


def self.carbs(name)
 grams = @list.each do |item|
    if item[0] == name
      return item[1]
    end
  end
  if grams == nil
    grams = "error"
  end
  return grams    
end
end

The list.csv file is as follows:

onion,13.75,0,0
carrot,11.375,0,0
cauliflower,19.375,0,0
cabbage,20.125,0,0
sw pepper,20,0,0
leek,7.5,0,0
mushroom,16.375,0,0
celery,33.25,0,0
apple,6.37,0,0
sweet potato,4.875,0,0
broccoli,14.8,0,0
red mill museli,1.52,0,0
mixed nuts,0,0,0.65
B. Sprouts,11,0,0
eggplant,16.66,0,0
quinoa,4.7,0,0
brown rice,4.33,0,0
sesame seed,0,0,4.5
sesame oil,0,0,4.655
pork chop,0,3.84,28.57
chick breast,0,3.22,27.77
lean turkey,0,4,100
ham,0,4.76,26.66

I edited my original code as follows:

require 'csv'

class Nutrition
    include Enumerable
    @list = CSV.read("./lib/list.csv")


    def self.carbs(name)
      result = @list.detect {|item| item|0| == name}
      if result.nil?
        result = "error"
      end
      result    
    end
end

Now, when I run rake test using the following testfile:

require './lib/nutrition.rb'
require "test/unit"
require 'csv'


class TestNutrition < Test::Unit::TestCase
  include Enumerable
  def test_carbs()
    assert_equal(Nutrition.carbs('onion'), "13.75")
    assert_equal(Nutrition.carbs('ham'),'0')
    assert_equal(Nutrition.carbs('sawdust'), 'error')
  end

end

I wind up with the following error message:

syntax error, unexpected == (SyntaxError) result = @list.detect {|item| item|0| == name} ^

However, when I run the following file, everything seems to work, I just can't seem to pass the rake test:

require 'csv'

class Nutrition
    include Enumerable
    @list = CSV.read("./lib/list.csv")


    def self.carbs(name)
      result = @list.detect {|item| item[0] == name}
      if result.nil?
        result = "error"
      end
      result    
    end
end

result = Nutrition.carbs('sawdust')
puts result
AntonySerio
  • 7
  • 4
  • You could make this work by writing `@list.each { |item| return item.last if item.first == name }; "error"; end`, but using `find` (aka `detect`) is the way to go. – Cary Swoveland Jan 18 '16 at 22:24

4 Answers4

2

You should use Enumerable#find instead of iterating through the array with each. If the element is not found, the return value will be nil. Otherwise, you will have it returned by find.

def self.carbs(name)
  grams = @list.find {|item| item[0] == name}

  if grams.nil?
    grams = "error"
  end

  grams
end
Geo
  • 93,257
  • 117
  • 344
  • 520
  • 1
    You could reduce that further to just `def self.carbs(name); @list.find {|item| item[0] == name} || "error"; end`; but +1 to Geo's approach. – konacaret Jan 18 '16 at 22:13
  • That looks like it should work, but I get back: lib/nutrition1.rb:10: syntax error, unexpected == result = @list.find {|item| item|0| == name} ^ – AntonySerio Jan 18 '16 at 23:50
0

grams will not be nil, even if the item is not found. However, if that test is reached, you can be sure that the item hasn't been found anyway because the function would have already returned if it had found something. So at that point, you can just return an error and forget testing grams.

John Sensebe
  • 1,386
  • 8
  • 11
0

After properly indenting your code, it is easy to see why the entire array is always returned:

class Nutrition

    def self.carbs(name)
        grams = @list.each do |item|
            if item[0] == name
                return item[1]
            end
        end
        if grams == nil
            grams = "error"
        end
        return grams    
    end

end

If name is never matched, then the last statement executed by self.carbs will be return grams, which will of course return the entire array.

EDIT Also, I would suggest a few changes in your design.

@ variables are instance variables (here's a good SO post explaining the difference between most kinds of Ruby variables), so it probably doesn't make sense using @list inside a class (i.e. static) method (self.carbs). You could either define carbs as a instance method or list as a class variable (@@list).

I did a little refactoring using the former option, and also replaced the each loop with a find:

class Nutrition
    def initialize
        @list = CSV.read("./lib/list.csv")
    end

    def carbs(name)
        return "error" if @list.nil?
        carb = (@list.find { |x| x[0] == name })
        carb.nil? ? "error" : carb[1]
    end
end

Using the latter option:

class Nutrition
    @@list = CSV.read("./lib/list.csv")

    def self.carbs(name)
        return "error" if @list.nil?
        carb = (@@list.find { |x| x[0] == name })
        carb.nil? ? "error" : carb[1]
    end
end
Community
  • 1
  • 1
Leo Brito
  • 2,053
  • 13
  • 20
  • I'm not sure if that would work. I tried defining list in initialize, but I always wound up with an empty array. The only way that I could build the array was to take list out of initialize. – AntonySerio Jan 18 '16 at 23:55
  • @AntonySerio I see no reason why it shouldn't work, but you could load the CSV in a class variable as well. I edited my answer with the code. – Leo Brito Jan 18 '16 at 23:58
0

The reason that this doesn't work as written is that you are assigning grams to the return value of calling each on @list: grams = @list.each.... You are then returning grams at the end: return grams

The return value of the each method is the original array, as described in the docs: http://ruby-doc.org/core-2.3.0/Array.html#method-i-each ("Returns the array itself.")

In your method, if an item IS found, the early return prevents the final line from running: return item[1]

dwenzel
  • 1,404
  • 9
  • 15