Mark Westling has already answered your immediate question, but since you mention you are new to Ruby, here are a couple of other style suggestions:
def subdirs
subdirs = Array.new
It is generally preferred to use literal syntax where possible, so the common way to express this would be subdirs = []
.
self.each do |x|
self
is the implicit receiver in Ruby, so you can just leave it off. (This is not Python, after all.) However, the primary purpose of code is communication, so if you believe, this communicates your intent better, leave it in.
Speaking of communication: x
is not a terribly communicative name, unless you are talking about points in a cartesian coordinate system. How about file
, or if you are uncomfortable with the Unix notion that directories are also files, the more neutral entry
? (Actually, the best one would probably be path
, but we will see soon how that might become confusing.)
My third suggestion is actually personal preference that is contrary to common Ruby style: common Ruby style dictates that one-line blocks are delimited with {
/}
and multiline blocks are delimited with do
/end
, just like you did. I don't like that, because it is an arbitrary distinction that doesn't convey any meaning. (Remember that "communication" thing?) So, I actually do things differently: if the block is imperative in nature and, for example, changes some global state, I use the keywords (because the block is actually do
ing some work) and if the block is functional in nature and just returns a new object, I prefer to use braces (because they look nicely mathematical). So, in this case, I would use braces.
if File.directory?(x)
As Mark already explained, you need to do something like File.directory?(File.join(path, entry))
here, where Dir#path
is a public attribute of the Dir
class returning the path that Dir.new
was initialized with.
Here you also see, why we didn't use path
as the name of the block parameter. Otherwise we would need to write File.directory?(File.join(self.path, path))
.
subdirs.push(x)
The canonical way to append an element to an array, or indeed append pretty much anything to anything in Ruby, is the <<
operator. So, this should read subdirs << entry
.
Array#push
is an alias of Array#<<
, which is mainly intended to let you use an Array
as a stack (in conjunction with Array#pop
).
end
end
return subdirs
Here is another disconnect between my personal style and common Ruby style: in Ruby, if there is no explicit return
, the return value is simply the value of the last expression. This means, you can leave off the return
keyword and common Ruby style says you should. I, however, prefer to use this similar to the block delimiters: use return
for methods that are functional in nature (because they actually "return" a value) and no return
for imperative methods (because their real return value is not what comes after the return
keyword, but what side-effects they have on the environment). So, like you, I would use a return
keyword here, despite common style.
It is also customary, to seperate the return value from the rest of the method body by a blank line. (The same goes for setup code, by the way.)
end
So, this is where we stand right now:
def subdirs
subdirs = []
each { |entry|
if File.directory?(File.join(path, entry))
subdirs << entry
end
}
return subdirs
end
As you can see, the if
expression really only serves to skip one iteration of the loop. This is much better communicated by using the next
keyword:
def subdirs
subdirs = []
each { |entry|
next unless File.directory?(File.join(path, entry))
subdirs << entry
}
return subdirs
end
As you can see, we managed to remove one level of nesting from the block structure, which is always a good sign.
This idiom is called a "guard clause", and is quite popular in functional programming, where a lot of languages even have guard constructs built in, but it is also used heavily in pretty much any other language on the planet, because it greatly simplifies control flow: the idea is analogous to a guard posted outside a castle: instead of letting the bad guys into the castle (method, procedure, function, block, ...) and then painfully trying to track their every move and constantly being afraid to miss something or lose them, you simply post a guard at the entrance of your castle (the beginning of your method, block, ...) who doesn't let them in to begin with (which jumps to the end of the procedure, returns early from the method, skips one iteration of the loop, ...). In Ruby, you can use raise
, return
, next
and break
for this. In other languages, even GOTO
is fine (this is one of those rare cases, where a GOTO
can actually clear up control flow).
However, we can simplify this even further, by recognizing the iterator pattern: you take a list (the directory entries) and then you "squash" that list down to a single object (the subdirs
array). To a category theorist, this screams "catamorphism", to a hardcore functional programmer "fold
", to a softcore functional programmer "reduce
", to a Smalltalk programmer "inject:into:
" and to a Rubyist "Enumerable#inject
":
def subdirs
return inject([]) { |subdirs, entry|
next subdirs unless File.directory?(File.join(path, entry))
subdirs << entry
}
end
inject
uses the return value of the previous iteration to seed the next one, that's why we have to return subdirs
, even if we are skipping an iteration, by using next subdirs
instead of plain next
(which would return nil
, so that on the next iteration subdirs
would be nil
and subdirs << entry
would raise a NoMethodError
.)
(Notice that I used curly braces for the block, even though the block actually modifies its argument. I feel this is still a "functional" block, though. YMMV.)
The last thing we can do is to recognize that what we are doing is just filtering (or in other words "selecting") elements of an array. And Ruby already has a method built in, which does exactly that: Enumerable#select
. Witness the single-line awesomeness that using the full power of Ruby generates:
def subdirs
return select { |entry| File.directory?(File.join(path, entry)) }
end
As a general tip: learn the wonders of Enumerable
. It is the workhorse of Ruby programming, similar to IEnumerable<T>
in .NET, dict
s in Python, lists in functional languages or associative arrays in PHP and Perl.