patternrubyMinor
Search for directories with a certain prefix that contain no files of a given type
Viewed 0 times
searchwithcontaintypefilesthatforprefixcertaingiven
Problem
These two functions search for directories with a certain prefix that contain no files of a given type.
The code works as intended, but here's a test case:
- Does the code adhere to Ruby standards and conventions?
- Am I doing something the hard way or the "non-Ruby" way?
- Should I use more/fewer/different comments?
def find_empties(directory, dirprefix, fsuffix)
empty_dirs = Array.new
Dir.chdir directory do
Dir["#{dirprefix}**/"].each do |subdir|
Dir.chdir subdir do
empty_dirs.push subdir if Dir["**.#{fsuffix}"].empty?
end
end
end
return empty_dirs
end
def find_empties_sorted(directory, dirprefix, fsuffix)
empties = find_empties(directory, dirprefix, fsuffix)
nums = Hash.new
non_numeric = Array.new
empties.each do |x|
# if starts with digits, trim leading zeroes and extract; else, nothing
num = x.sub(/^#{Regexp.escape dirprefix}0*(\d+).*/, '\1')
is_num = num !~ /\D/
if is_num
n = Integer num
nums[n] = Array.new unless nums[n]
nums[n].push x
else
non_numeric.push x
end
end
result = Array.new
nums.sort.each do |k, v|
v.each do |x|
result.push x
end
end
result.concat non_numeric.sort
result
endThe code works as intended, but here's a test case:
$ ls
emptyfinder.rb
emptyrunner.rb
$ cat emptyrunner.rb
require_relative 'emptyfinder.rb'
puts find_empties_sorted(Dir.pwd, 'e', 'txt')
$ mkdir testcase
$ cd testcase
$ mkdir e1 e2 e003 e004 e4 e5 e9 e10 e11 empty emptier emptynot
$ touch emptynot/file.txt
$ touch empty/not_a_txt.csv
$ ruby ../emptyrunner.rb
e1/
e2/
e003/
e004/
e4/
e5/
e9/
e10/
e11/
emptier/
empty/
$
Solution
Looks alright.
Your style is more spread out than is usually the case in ruby (though that's not necessarily a bad thing.) For example
could as well be one line sans assignments, without loosing clarity.
Instead of declaring
Instead of iterating through the array v of sorted nums and pushing each item to result, you can just do
Be aware that you can initialise arrays and hashes as literals, e.g.
Your level of commenting seems appropriate to me, i.e. explaining what the code is meant to do (or why but not how) when it's not obvious from the code itself.
Your style is more spread out than is usually the case in ruby (though that's not necessarily a bad thing.) For example
is_num = num !~ /\D/
if is_num
...could as well be one line sans assignments, without loosing clarity.
if num !~ /\D/ # is a numberInstead of declaring
nums to be a Hash, then having an if statement to check if a pair needs initialised with an array, you can just declare the hash with a block that it then uses to default all values to new arrays like so nums = Hash.new { |h,k| h[k] = [] }.Instead of iterating through the array v of sorted nums and pushing each item to result, you can just do
result.concat v. Sometimes for simple logic, denser one liners are arguably easier to read in ruby: e.g. nums.sort.each { |k, v| result.concat v }Be aware that you can initialise arrays and hashes as literals, e.g.
a = [] and this is sometimes prefered.Your level of commenting seems appropriate to me, i.e. explaining what the code is meant to do (or why but not how) when it's not obvious from the code itself.
Code Snippets
is_num = num !~ /\D/
if is_num
...Context
StackExchange Code Review Q#40655, answer score: 8
Revisions (0)
No revisions yet.