Scope problem (?) in implementing Design Patterns in Ruby

[RichardOnRails]

Hi,

I'm hoping that employing "Design Patterns in Ruby" will lead to less
coding errors and more easily maintained code. I'm stuck with the Not
pattern in the File Finding pattern in Chapter 15.

I've posted my code and output in http://www.pastie.org/1889586 and
http://www.pastie.org/188968, respectively. Any idea about the cause
of the reported syntax error?

Thanks in Advance,
Richard

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

I think the second link points to the wrong pastie.

I can see a couple of problems with the code, first of which is that you are
setting up Expression.new to require an argument but not passing it an
argument when you call All_basic.new, etc.

More than design patterns (which I think are a little much here), I would
suggest looking into Ruby's higher order functions like reduce and map. They
make your code a lot simpler. This is an alternate implementation I did. Let
me know what you think:

http://www.pastie.org/1889838

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

Correction: My bad, your version of All_basic doesn't inherit from
Expression like I thought it would from the pattern it follows.

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

One more suggestion: if you're going to build a hierarchy, might as well
work the parent as much as possible. My final implementation would probably
look something like this:

http://www.pastie.org/1889969

On Wed, May 11, 2011 at 1:51 PM, David Jacobs <deve...@wit.io> wrote:

[7stud --]

RichardOnRails wrote in post #998059:
>

1) Your case statement syntax doesn't work in ruby 1.9.2:

prog.rb:4: syntax error, unexpected ':', expecting keyword_then or ','
or ';' or '\n'

You have to use 'then' in place of a colon.

2) Next, I get this error:

prog.rb:67:in `evaluate': uninitialized constant Not::All (NameError)
from prog.rb:74:in `<main>'

which relates to this code:

class Not < Expression
def initialize(expression)
@expression = expression
end

def evaluate(dir)
all = All.new.evaluate(dir)
other = @expression.evaluate(dir)
all - other
end
end

In ruby, constants are looked up like directories and files. When you
are inside the Not class (which is a module), the 'directory' you are in
for constant lookups is the 'Not' directory. When you write All.new,
because the name All is not preceded by a directory name, ruby looks in
the current 'directory' for the constant All. The current directory is
Not, so ruby is looking for Not::All, i.e. the 'file' All in the
'directory' Not. However, All is not defined inside Not, so you get an
error. In fact, there is not constant named All defined anywhere in
your program.

--
Posted via http://www.ruby-forum.com/.

[7stud --]

7stud -- wrote in post #998081:

> RichardOnRails wrote in post #998059:
>>
>
> 1) Your case statement syntax doesn't work in ruby 1.9.2:
>
> prog.rb:4: syntax error, unexpected ':', expecting keyword_then or ','
> or ';' or '\n'
>
> You have to use 'then' in place of a colon.
>
> 2) Next, I get this error:
>
> prog.rb:67:in `evaluate': uninitialized constant Not::All (NameError)
> from prog.rb:74:in `<main>'
>
> which relates to this code:
>
> class Not < Expression
> def initialize(expression)
> @expression = expression
> end
>
> def evaluate(dir)
> all = All.new.evaluate(dir)
> other = @expression.evaluate(dir)
> all - other
> end
> end
>

> In ruby, constants are looked up like directories and files (or if you
> prefer constants are 'lexically scoped'). When you

> are inside the Not class (which is a module), the 'directory' you are in
> for constant lookups is the 'Not' directory. When you write All.new,
> because the name All is not preceded by a directory name, ruby looks in
> the current 'directory' for the constant All. The current directory is
> Not, so ruby is looking for Not::All, i.e. the 'file' All in the
> 'directory' Not. However, All is not defined inside Not, so you get an

> error. In fact, there is no constant named All defined anywhere in
> your program, so the error is more serious than a scope problem.
>
> If you are inside a class/module and you need to access a class at the
> top level, you do this:
>
> class All
> def greet
> puts 'hi'
> end
> end
>
>
> class Dog
> def do_stuff
> ::All.new.greet #<*****
> end
> end
>
> Dog.new.do_stuff #=>hi
>

In fact, that is unnecessary:

class All
def greet
puts 'hi'
end
end

class Dog
def do_stuff
All.new.greet #<*****
end
end

Dog.new.do_stuff #=>hi

I guess the lookup actually starts at the toplevel. So your error is a
result of not defining the constant All at the toplevel, and ruby
obfuscates the error by telling you that All is not defined inside the
Not module, giving you the error message: Not::All doesn't exist.

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

I think the lookup starts at the innermost scope but since All isn't defined
in Not, it reaches the top-level.

[7stud --]

7stud -- wrote in post #998090:

>
> I guess the lookup actually starts at the toplevel.

Well, that's not true either. If I define two All classes: one at the
top level and one inside a module,

class All
def greet
puts 'All#greet'
end
end

module C
class All
def greet
puts "C::All#greet"
end
end

class Dog
def do_stuff
All.new.greet #<*****
end
end

end

C::Dog.new.do_stuff

--output:--
C::All#greet

Then if I delete C::All:

class All
def greet
puts 'All#greet'
end
end

module C

class Dog
def do_stuff
All.new.greet #<*****
end
end

end

C::Dog.new.do_stuff

--output:--
All#greet

..which doesn't seem like that should work.

[7stud --]

David Jacobs wrote in post #998093:

> I think the lookup starts at the innermost scope but since All isn't
> defined
> in Not, it reaches the top-level.

Yes, you are right:

1)
===
Constants defined within a class or module may be accessed unadorned
anywhere within the class or module.

(Programming Ruby)
===

2)
===
Constants declared outside of a class or module are assigned global
scope.

(http://www.techotopia.com/index.php/Ruby_Variable_Scope#Ruby_Constant_Scope)
===

So it's a case of the inner All hiding the global All. And you can use
the :: prefix to leap over an inner scope constant that hides a toplevel
constant:

class All
def greet
puts 'All#greet'
end
end

module C
class All
def greet
puts "C::All#greet"
end
end

class Dog
def do_stuff
::All.new.greet #<*****
end
end

end

C::Dog.new.do_stuff

--output:--
All#greet

--
Posted via http://www.ruby-forum.com/.

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

>
> ...which doesn't seem like that should work.

Why not?

[RichardOnRails]

> I think the second link points to the wrong pastie.
It sure does. I apologize for that error. My results are at
http://www.pastie.org/1889681

> This is an alternate implementation I did. Let
> me know what you think:
>
> http://www.pastie.org/1889838

I downloaded your example, removed a few spurious "end" statement and
added a "p" to get the Regexp class name.
It ran perfectly as far as I can see, but I'd have to format the
output to confirm that.
The code looks great, but I'll await a real comparison until I get
the book's code working

> I can see a couple of problems with the code, first of which is that you are
> setting up Expression.new to require an argument but not passing it an
> argument when you call All_basic.new, etc.

Thanks for this possible solution. I'll try to correct that after I
look at the other posts I've been lucky enough to get.

Best wishes,
Richard

[RichardOnRails]

Hi David,

Assuming I'm responding to the correct you version works perfectly, as
evidenced by results below gotten after I decorated your code with
puts versions.

The Design Patterns author, Russ Olson, is aims to support a neat DSL
to support file search with arbitrary logical combinations of the
fundamental search elements ... which is my adopted goal. So I've got
to master his approach to implementing this search language before I
can really weigh approaches.

7stud has responded with the key thing I saw but couldn't figure out
how to address the problem, so I've got to follow up on that now.

Thanks for very detailed responses in intrinsic Ruby education.

Best wishes,
Richard

======== Output ==========
>ruby DavesTempFilesDirs_2.rb

files.select &all
==========
AlternativeFileSearch.rb
DavesTempFilesDirs_1.rb
DavesTempFilesDirs_2.rb
FindFilesDirs-01.rb
FindFilesDirs-02.rb
FindFilesDirs.rb
TempFindFilesDirs.rb
TestDoc@.txt
TestFolder
TestFolder/TestDoc.txt

files.select &filename(/-01/))
==========
FindFilesDirs-01.rb

files.select &writable
==========
AlternativeFileSearch.rb
DavesTempFilesDirs_1.rb
DavesTempFilesDirs_2.rb
FindFilesDirs-01.rb
FindFilesDirs-02.rb
FindFilesDirs.rb
TempFindFilesDirs.rb
TestFolder/TestDoc.txt

files.select &_not(writable)
==========
TestDoc@.txt
TestFolder
>Exit code: 0

[RichardOnRails]

Hi 7Zip and David,

Thanks very much to you both for hanging in there with me.

I hacked up a working version (agnostic for 1.8.6/1.9.2) here:
http://www.pastie.org/1893405,
with results here: http://www.pastie.org/1893420

I apologize for having screwed a Pastie URL last time.

I left comments in code for the hacks I made. When I first
encountered the problem, I made a feeble attempt at prefixing :: for
All but I failed to take note of the fact that the All definition was
subordinate to Expression. That was stupid on my part, but I blame it
on the fact that I'm 77, have been retired from programming for 7
years and have only got serious about learning Ruby/Rails about a year
ago. It's been a struggle; I used to be faster on the uptake.

BTW, David Black has a very nice exposition of lookup for methods and
classes on page 98 et seq of his "The Well-Grounded Rubyist", one of
my "bibles".

I don't know how my hacks will playout for the parsing scheme in
"Design Patterns in Ruby". I going to follow it until this newsgroup
and/or I discover it's a fool's errand.

With my very best wishes to you both and thanks for your generous
insights,
Richard

[David Jacobs]

Hi Richard,

Happy to help.

I would definitely encourage you to not only look at design patterns (which are okay, but often overkill or already built in to Ruby) but also to Ruby's standard features to reduce code errors.

Ruby is a rich language. It has a lot of really well-crafted methods that take care of a most of the tedium of coding, and for that reason the standard library is well worth learning.

For example, in your code, instead of importing 'find' and using it to look at directories, use Ruby's built-in Dir module. Dir['**/*'] will get you all subdirectories and files recursively.

Second, look into higher order functions. They let you change code from this:

def evaluate(dir, arg)
results = []
Dir['**/*'].each do |p|
next unless File.file? p
name = File.basename(p)
case arg
when String
results << p if File.fnmatch(@arg, name)
when Regexp
results << p if @arg.match(name)
end
end
results
end

Into this:

def match?(str_or_re, file)
str_or_re.is_a?(String) ? File.fnmatch(str_or_re, file) : str_or_re =~ file
end

def evaluate(dir, arg)
Dir['**/*'].select {|f| File.file?(f) and match?(arg, f) }
end

Any time you see this pattern ...

my_temporary_variable = []

my_collection.each do |elem|
my_temporary_variable << elem if some_condition
end

return my_temporary_variable

.. you know that higher order functions would probably be a better solution. :)

Hope that helps, and good luck learning this beautiful language.

Cheers,
David

Any time you see this pattern in your code, you should automatically think

[7stud --]

David Jacobs wrote in post #998417:

> Hi Richard,
>
> Happy to help.
>
> I would definitely encourage you to not only look at design patterns
>

Hard to do when you are reading a book on Design Patterns, and you are
trying to understand one of the examples. I'm also currently reading
the same book as RichardOnRails, but I'm not quite that far along.

[David Jacobs]

Hard to do when you are reading a book on Design Patterns, and you are
> trying to understand one of the examples. I'm also currently reading
> the same book as RichardOnRails, but I'm not quite that far along.

I've read the book. It's good but I think it's healthy to understand how design patterns aren't always necessary in Ruby like they are in Java. I'm just pointing out other, potentially more effective ways to write re-usable code.

Cheers,
David

[7stud --]

Is it a higher order function because it calls a global method?

[David Jacobs]

No, sorry for the confusion, the higher order function transformation was this:

results = []
Dir['**/*'].each do |p|

next unless (... ...)
results << p if (... ...)
end
results

Into this:

Dir['**/*'].select {|x| (... ...) }

Other good examples are Array#map, Array#reduce and Array#grep.

PS I apologize for the indentation, my mail client is acting up.

[RichardOnRails]

Hi Dave,

I checked out your Version 2. Added puts stuff and restriction to
names of files, which should probably be another one of your methods
instead of my hard-coding.
Expanded code: http://www.pastie.org/1895917
Output: http://www.pastie.org/1895932

I have only one question: I put a space between the & and the method
following it; the code broke. Is the construct you used documented on-
line somewhere? I've peeked at lambda documentation so I've got the
drift but I haven't used it in any of my code so far.

I copied your memo on the approach and plan to review it and use/
expand your code after I finish getting Russ Olson's code working or
giving up on it. To his credit, he posted his code on-line so I can
avoid typos and he included an apparently thorough set of unit tests.
So I've got a lot of studying to do.

Again, thanks for your guidance and great ideas,
Richard

[David Jacobs]

[Note: parts of this message were removed to make it a legal post.]

Hi Richard,

The & has to be adjacent to "all" or "filename" to act as expected. The &
can mean one of two related things when you pass it to a method:

1. It can mean "pass the lambda expression that this variable points to into
a block". That makes the following to snippets equivalent:

# Snippet 1
filter_function = lambda {|x| x == 2 }
[1, 2, 3, 4, 2].select &filter_function

# Snippet 2
[1, 2, 3, 4, 2].select {|x| x == 2 }

2. It can mean "pass the instance method named by a symbol, and treat it as
a block".

That makes these two equivalent:

# Snippet 1
[1, 2, 3, 4, 2].select {|x| x.even? }

# Snippet 2
[1, 2, 3, 4, 2].select(&:even?)

So what my code is doing is calling the all method, for example, and that
method's sole job is to return a lambda that can then be passed in per the
above. You don't have to wrap the lambda in a method like I did. You could
also just say:

all = lambda {|x| x }

I hope that makes things a little clearer!

Cheers,
David

[Ryan Davis]

On May 11, 2011, at 10:20 , RichardOnRails wrote:

> I'm hoping that employing "Design Patterns in Ruby" will lead to less
> coding errors and more easily maintained code. I'm stuck with the Not
> pattern in the File Finding pattern in Chapter 15.

Design patterns do not lead to less coding errors nor more easily maintained code.

Design patterns simply provide developers with a shared vocabulary to oft-used structures. It allows them to more easily communicate. If you're coding in a vacuum it won't help (from that perspective).

Not to say that it isn't worthwhile to read more code. That's always worthwhile.

[7stud --]

RichardOnRails wrote in post #998460:

>
> I have only one question: I put a space between the & and the method
> following it; the code broke. Is the construct you used documented on-
> line somewhere? I've peeked at lambda documentation so I've got the
> drift but I haven't used it in any of my code so far.
>

Your bible, "The Well-Grounded Rubyist" uses the &:meth_name construct a
lot, and David Black says that the '&' in '&:meth_name' does two things:

1) It calls the specified object's to_proc() method. And Symbol#to_proc
is defined like this:

class Symbol
def to_proc
proc{|obj, *args| obj.send(self, *args)}
#self=:meth_name
end
end

That just creates a function that takes an object as an argument and
sends() the :meth_name to the object, i.e. the function calls the
:meth_name method on the object.

2) The '&' tells ruby to convert the Proc to a block.

[RichardOnRails]

Hi again, Dave,

> I hope that makes things a little clearer!

Crystal! as Kaffee said to Col. Jessep during their Gitmo meeting (A
Few Good Men)

Thanks for illuminating another area of Ruby where my proficiency
needs enhancement :-)

Best,
Richard

[Steve Klabnik]

[Note: parts of this message were removed to make it a legal post.]

>

> 1) It calls the specified object's to_proc() method. And Symbol#to_proc
> is defined like this:
>

Small note, and some history:

This was originally introduced in ActiveSupport, where that was in fact the
definition. But performance was terrible. It was added to 1.8.7 & 1.9, but
is implemented in C, and is a bit more efficient. The semantics are the same
though.

[RichardOnRails]

Hi 7stud,

I just your the response you posted a week of two ago, for which I am
grateful

> Your bible, "The Well-Grounded Rubyist" uses the &:meth_name construct a

> lot, and David Black says that...

I had looked at the indices of several books for a reference to the
prefix &, to no avail. So I abandoned that idea and thus never
checked Black's book.

Thanks to your note, I see that Black has indexed two dense sections
related to the prefixed &, so I've got them on my To-Do list for
study.

Thanks again for taking the trouble to directing me to Black's
coverage.

Best wishes,
Richard

[RichardOnRails]

On May 12, 9:56 pm, David Jacobs <develo...@wit.io> wrote:

> Second, look into higher order functions. They let you change code from this:

Hi Dave,

I just want to let you know your tutelage on this thread has not gone
in vain.

1. I wanted to return a list (i.e. array) of filenames rather than
concatenated strings. Maybe parsing the concatenated strings would
have gotten me my lists, but I want the arrays to be the direct output
(in part just to see if I understood your code well enough to do it.)

2. I wanted to do it by passing a search pattern to Filename.new and
a directory to the related expression.

These changes are displayed at http://www.pastie.org/1969301

I'm continuing to work on embracing the higher-order methods you
advocated.

Best wishes,
Richard

[David Jacobs]

Hi Richard,

Glad the advice helped!

Probably a more concise (and robust) way to get the full pathname of your Dir[] results would be:

full_name = File.expand_path(path)

This has the advantage of expanding '~' to '/home/richard' (or whatever).

If you add in higher order functions *here* (I'm just saying this to reinforce the earlier concept along with the above method), you go from this:

array = []
Dir.chdir(dir) do
Dir['**/*'].each do |path| # Added each & block
full_name = File.join(dir, path)
array << path
end
end
array

To this:

Dir.chdir(dir) do
Dir['**/*'].map {|path| File.expand_path(path) }
end

[RichardOnRails]

On May 25, 6:30 pm, David Jacobs <develo...@wit.io> wrote:
> Hi Richard,
>
> Glad the advice helped!
>
> Probably a more concise (and robust) way to get the full pathname of your Dir[] results would be:
>
> full_name = File.expand_path(path)
>
> This has the advantage of expanding '~' to '/home/richard' (or whatever).
>
> If you add in higher order functions *here* (I'm just saying this to reinforce the earlier concept along with the above method), you go from this:
>
> array = []
> Dir.chdir(dir) do
> Dir['**/*'].each do |path|  # Added each & block
>  full_name = File.join(dir, path)
>  array << path
> end
> end
> array
>
> To this:
>
> Dir.chdir(dir) do
> Dir['**/*'].map {|path| File.expand_path(path) }

> endOn Tuesday, 24 May 2011 at 11:35 pm, RichardOnRails wrote:
>
> On May 12, 9:56 pm, David Jacobs <develo...@wit.io> wrote:
>
>
>
>
>
>
>
>
>
> > > Second, look into higher order functions. They let you change code from this:
>
> > Hi Dave,
>
> > I just want to let you know your tutelage on this thread has not gone
> > in vain.
>
> > 1. I wanted to return a list (i.e. array) of filenames rather than
> > concatenated strings. Maybe parsing the concatenated strings would
> > have gotten me my lists, but I want the arrays to be the direct output
> > (in part just to see if I understood your code well enough to do it.)
>
> > 2. I wanted to do it by passing a search pattern to Filename.new and
> > a directory to the related expression.
>

> > These changes are displayed athttp://www.pastie.org/1969301

>
> > I'm continuing to work on embracing the higher-order methods you
> > advocated.
>
> > Best wishes,
> > Richard

Hi David,

Thanks for your additional improvement. Shrinking 8 lines down to 3
reduces the likelihood of error 62.5% I'd say. I like those odds|

I'm presently going through David Black's treatise as fast as I can
internalize the stuff. Specifically, I just found out the the number
of Enumerable methods went from 26 to 47 as I switched my WindowsXP-
Pro's PATH from Ruby 1.8.6 to 1.9.2. I just ran into a minor problem
that I'm going to post on a separate thread because I don't want
presume to hang on your coat-tails.

Best wishes,
Richard