Using Each to Iterate

[ixnay]

Greetings all.

Hopefully I've described this adequately and someone can see what I'm
missing. I have a feeling it's obvious. I've been using the Song and
SongList examples from the Pragmatic Programmer's guide, but I'm stuck on
one thing.

I have a class called StepList and in that class I have an each method as
such:

class StepList
def initialize
@guides = Array.new
end
def each
@guides.find { |guide| guide.filter }
end
end

If I put a "puts" in front of the guide.filter, I see that this is working
correctly by just grabbing the values I want. However, the results are never
passed back. For example, I have this:

$stepList.each { |thisFilter|
puts "test"
puts thisFilter
}

What I'm trying to do is get the value of each filter and have that get
placed in thisFilter each time through the loop. What happens is that
nothing happens! I do not even see the "test" text get printed out. Yet, it
seems the each must be looping because the guide.filter in my my each method
does return values if I put a puts in front of it.

Hopefully I've provided enough information here. Any help would be
appreciated.

- Jeff

 

[Mariano Kamp]

Hi Jeff,

[..]

I have a class called StepList and in that class I have an each
method as
such:

class StepList
def initialize
@guides = Array.new
end
def each
@guides.find { |guide| guide.filter }
end
end

Not sure what you are trying to accomplish here?
"each" should return each filtered element? guide.filter returns
true, when some conditions are met?

Currently each would return the first element in guides that match
"guide.filter". It also wouldn't make any use of a block you pass in.

If I put a "puts" in front of the guide.filter, I see that this is
working
correctly by just grabbing the values I want.

Could you provide the full source code and the output of the running
program?
The puts would show you all the elements it will try to filter and as
puts returns nil (think false) the whole list of guides will be tried
before returning nothing.

However, the results are never
passed back. For example, I have this:

$stepList.each { |thisFilter|
puts "test"
puts thisFilter
}

See above ... Your each doesn't care for a block... You don't call
yield anywhere...

What I'm trying to do is get the value of each filter and have that
get
placed in thisFilter each time through the loop. What happens is that
nothing happens! I do not even see the "test" text get printed out.
Yet, it
seems the each must be looping because the guide.filter in my my
each method
does return values if I put a puts in front of it.

Still not sure what you are trying to do?
You want to be called with all the guides that match a certain criteria?
I don't have the pickaxe handy, please refresh my memory what you are
trying to do.

Cheers,
Mariano

 

[MonkeeSage]

As Mariano said, you don't have a yield anywhere and are not calling
the block to each, so each is not acting like an iterator and your
block is unused. You need something like:

@guides.find { |guide| yield guide.filter } # note the yield

Regards,
Jordan

 

[Jeff Nyman]

[Sending this from another account, so I'm sorry if this double-posts.]

With all the responses I got here, I figured it out. Thanks to all of you.
I'll put the relevant source here and what I did, just in case this helps
others.

===============

First, a bit of explanation. I have a Step class. I also have a StepList
class. (This is similar to the Song and SongList class in the Pickaxe book.)

The idea is that when a new step is created, I do so like this:

$stepList.append(Step.new(firstStep, thisStep, thisFilter))

So here are the classes:

<code>
class Step
attr_reader oint1, oint2, :filter

def initialize(point1, point2, filter=nil)
@point1 = point1
@point2 = point2
@filter = filter
end

end

class StepList
def initialize
@guides = Array.new
end

def append(thisGuide)
@guides.push(thisGuide)
self
end

def filter(thisGuide)
@guides.find { |guide| thisGuide == guide.filter }
end

def [](index)
@guides[index] if index.kind_of?(Integer)
end

def each
@guides.find { |guide| guide.filter }
end
end
</code>

Then I have $stepList referencing a StepList object. When I have added all
the steps and just do a print of $stepList, I get this:

#<StepList:0x2f37954 @guides=[#<Step:0x2a9ad10 @filter="first after
202G_OrdAdd,203G_OrdUp
dateFirst", @point2="203G_OrdUpdate", @point1="202G_OrdAdd">,
#<Step:0x2a99e88 @filter="la
st,203G_OrdUpdateLast", @point2="203G_OrdUpdate", @point1="202G_OrdAdd">,
#<Step:0x2a96a94
@filter="last,203G_OrdUpdateLast", @point2="203G_OrdUpdate",
@point1="203G_OrdUpdate">]>

So what I wanted to do is filter through all the Step objects that are in
the StepList and then check if they have a filter set.

So here was the logic I had:

$stepList.each { |thisFilter|
puts thisFilter
}

What my hope was is that the "each" method of my StepList class would return
me the filters. And it did -- if I changed my method like this:

def each
@guides.find { |guide| puts guide.filter }
end

(or if I used "yield" as some had suggested). With the puts or yield in
place, I saw this when the code for the $stepList.each is run:

first after 202G_OrdAdd,203G_OrdUpdateFirst
last,203G_OrdUpdateLast
last,203G_OrdUpdateLast

Those are definitely the filters. So what I was hoping was that each of
those would, in turn, get sent to thisFilter in my each loop. That appeared
to not happen. If I changed the 'each' method to a 'selection' method
(Paul's idea), I got this if I had yield in place in the method:

../classes.rb:118:in `selection': no block given (LocalJumpError)

However, if I did not have yield in place, I got this:

undefined method `each' for #<Step:0x2a9a98c>

That seemed to make sense since I needed an 'each' method for my Step class.
So I added this 'each' method to the Step class:

def each
yield @filter
end

That seems to do the trick perfectly.

- Jeff

 

[Eric Schwartz]

A very minor comment:

def [](index)
@guides[index] if index.kind_of?(Integer)
end

I think you're doing yourself a disservice here. Remember duck
typing! If you pass something that's not a kind_of?(Integer), then
you don't raise any errors at all, and users of your code, even if
that's just you, won't get a warning that they did something silly.
The result (or in this case, lack of one) might percolate a ways up
the call chain until something bad happens. Much better to just

def [](index)
@guides[index]
end

or even

def [](index)
@guides[index.to_i]
end

That way, if you pass something that can't be converted into an
Integer easily, you'll find out closest to where the problem actually
occurs.

-=Eric

 

[Mariano Kamp]

Hi Jeff,

With all the responses I got here, I figured it out. Thanks to all
of you.
I'll put the relevant source here and what I did, just in case this
helps
others.

I have the feeling that you didn't figure it out completely yet.

A few small things:

def append(thisGuide)
@guides.push(thisGuide)
self
end

Btw. Did you know that you could use "<<" as a method name?

def filter(thisGuide)
@guides.find { |guide| thisGuide == guide.filter }
end

Find will only return the first occurrence. find_all will return all
occurrences.

def [](index)
@guides[index] if index.kind_of?(Integer)
end

Btw. (2) ... you can check if the index is of the right type,
probably to check for a coding error, but then you just return nil.
So that the error can slip silently. **If** you want to do this
checking here, instead of in the unit tests, you might want to raise
an exception, but then this would not be necessary as Array would
already do that for you.

def each
@guides.find { |guide| guide.filter }
end

Here again, you would just find the first occurrence. So the method
would be more accurately named "first" or something. Especially since
"each" has a well known meaning to Ruby programmers. If you encounter
an each method in someone's code you would think that you can pass in
a block and this block will be called for *each* element.

Sounds complicated? It is not. Really easy stuff. If you implement
each, you know about the internal structure of the collection you
manage and you know how to make it happen that you can call "yield
element" for each of the elements in your collection.

Cheers,
Mariano

 

[Jeff Nyman]

I have the feeling that you didn't figure it out completely yet.

You are correct, as it turns out.

I did need the find_all. What's interesting is I find that I can do the
following:

(1) Have my selection method in the StepList class look like this:

def selection
@guides.find_all { |guide| guide }
end

(2) Have my 'each' method in the Step class look like this:

def each
end

(3) Cycle through the elements like this:

$stepList.selection.each { |thisStep|
puts thisStep
}

If I run that above code (from (3)), I get this:

Step Guide: 202G_OrdAdd, 203G_OrdUpdate, first after
202G_OrdAdd|203G_OrdUpdateFirst
Step Guide: 202G_OrdAdd, 203G_OrdUpdate, last|203G_OrdUpdateLast
Step Guide: 203G_OrdUpdate, 203G_OrdUpdate, last|203G_OrdUpdateLast

Here the last part (after the second comma) is actually the filter part I
want. With this, however, I can at least parse the string and get to that.

I should note that to get the above output, it was necessary to have a to_s
method in the Step class. Otherwise, I would get this when the
'selection.each' was run:

#<Step:0x2a9b15c>
#<Step:0x2a9a338>
#<Step:0x2a995dc>

I think what was (perhaps still is) throwing me off is that I have an object
(Step) being stored within another object (StepList).

- Jeff

 

[Mariano Kamp]

Hi Jeff,

I did need the find_all. What's interesting is I find that I can do
the
following:

(1) Have my selection method in the StepList class look like this:

def selection
@guides.find_all { |guide| guide }
end

(2) Have my 'each' method in the Step class look like this:

def each
end

Actually you can't. You are just not calling this each method.
=> nil

You see, it will just return nil. So it must be something different
you are calling.

(3) Cycle through the elements like this:

$stepList.selection.each { |thisStep|
puts thisStep
}

You call each on the returned object of "selection". And that is an
array ... So you are calling Array#each.

Btw. It is perfectly legal to use camelCase for variable- and method
names, like you would do in Java, and a lot of people do it, but most
of the time I see people using "camel_case". thisStep -> this_step ...

It seems that you don't need your own "each" after all.

If I run that above code (from (3)), I get this:

Step Guide: 202G_OrdAdd, 203G_OrdUpdate, first after
202G_OrdAdd|203G_OrdUpdateFirst
Step Guide: 202G_OrdAdd, 203G_OrdUpdate, last|203G_OrdUpdateLast
Step Guide: 203G_OrdUpdate, 203G_OrdUpdate, last|203G_OrdUpdateLast

Here the last part (after the second comma) is actually the filter
part I
want. With this, however, I can at least parse the string and get
to that.

Uuh. Don't do that.
I looked at your code again, but didn't find the part where you add
the elements.

I guess you call this method:

def append(thisGuide)
@guides.push(thisGuide)
self
end

But what do you add? I would assume an instance of Step?

I should note that to get the above output, it was necessary to
have a to_s
method in the Step class. Otherwise, I would get this when the
'selection.each' was run:

#<Step:0x2a9b15c>
#<Step:0x2a9a338>
#<Step:0x2a995dc>

I think what was (perhaps still is) throwing me off is that I have
an object
(Step) being stored within another object (StepList).

Yeah, maybe. In this particular case I would like to see the to_s
method what the information after the 2nd comma is.
I take it that you want to output a particular attribute of Step, but
you ask "puts" to output the whole object:

$stepList.selection.each { |thisStep|
puts thisStep
}

So what you could do is this: "puts thisStep.attributename"... As
you've put attr_reader oint1, oint2, :filter in your Step class
you will be able to access these three attributes this way.

Bonus Answer (if you're choking on the stuff above, ignore this):
I assume that this "puts" thing is a step to the actual goal. You
probably just want all the attribute values in an Array? To do that
you could use the following code:

$steps.selection.collect {|step| step.point1}

And the code will return an array of point1s. Collect will create a
new array with all of the return values of the block. And you get
this feature for free as Array implements the each method. That
enables Array to include the Enumerable module. This module only
needs this one method, each, and will provide other methods for you
that each use each (no pun intended). Beautiful? Yes! "find" and
"find_all" from above work the same way. "find" for example call each
as long as the block will return "true". Have a look at http://
corelib.rubyonrails.org/classes/Enumerable.html.

Cheers,
Mariano

 

[Jeff Nyman]

Uuh. Don't do that.
I looked at your code again, but didn't find the part where you add the
elements.

I guess you call this method:

def append(thisGuide)
@guides.push(thisGuide)
self
end

But what do you add? I would assume an instance of Step?

Correct. Basically, I do this when adding to the list:

$stepList.append(Step.new(firstStep, thisStep, thisFilter))

Here 'firstStep', 'thisStep' and 'thisFilter' are just text values that are
read in from an XML file. (Incidentally, most of this is copied largely from
the Pickaxe book. I hijacked the examples of Song and SongList since they
seemed fairly close to what I needed to do.)

Yeah, maybe. In this particular case I would like to see the to_s method
what the information after the 2nd comma is.

The 'to_s' method I have in the Step class looks like this:

<code>
class Step
....
def to_s
if (@filter != nil && @filter != "")
"#@point1, #@point2, #@filter"
else
"#@point1, #@point2"
end
end
end
</code>

So it's nothing really fancy.

Beyond this, I will try the suggestions you bring up here.

- Jeff

 

[Mariano Kamp]

Hi Jeff,

The 'to_s' method I have in the Step class looks like this:

<code>
class Step
...
def to_s
if (@filter != nil && @filter != "")
"#@point1, #@point2, #@filter"
else
"#@point1, #@point2"
end
end
end
</code>

Then the information printed after the 2nd comma is the filter and
you could instead write in your block: "puts step.filter". No String
manipulation needed.

Have fun.

Cheers
Mariano