Too much nesting in ruby?

Question

Surely there must be a better way of doing this:

File.open('Data/Networks/to_process.txt', 'w') do |out|
  Dir['Data/Networks/*'].each do |f|
    if File.directory?(f)
      File.open("#{f}/list.txt").each do |line|
        out.puts File.basename(f) + "/" + line.split(" ")[0]
      end
    end
  end
end

Cheers!

You could at least provides comments to describe what you are doing there. — Emiliano Poggi, Aug 23 '11 at 13:50
Data/Networks/___/list.txt contains a list of lines in the format: "SOMEHASH SOMEOTHERVALUE". I want to compile a list of "___/SOMEHASH" to Data/Networks/to_process.txt — Steve C, Aug 23 '11 at 13:53

DNNX · Answer 1 · 2011-08-23T14:15:04.953

8

You can rid of 1 level of nesting by utilizing Guard Clause pattern:

File.open('Data/Networks/to_process.txt', 'w') do |out|
  Dir['Data/Networks/*'].each do |f|
    next unless File.directory?(f)
    File.open("#{f}/list.txt").each do |line|
      out.puts File.basename(f) + "/" + line.split(" ")[0]
    end
  end
end

See Jeff Atwood's article on this approach.

edited Aug 23 '11 at 14:15

answered Aug 23 '11 at 14:07

DNNX

6,215
2
27
33

score 6 · Answer 2 · answered Aug 23 '11 at 14:06

6

IMHO there's nothing wrong with your code, but you could do the directory globbing and the check from the if in one statement, saving one level of nesting:

Dir.glob('Data/Networks/*').select { |fn| File.directory?(fn) }.each do |f|
  ...
end

answered Aug 23 '11 at 14:06

Michael Kohl

66,324
14
138
158

Expressing it as a series of transforms like this is often more readable than a series of nested statements. – tadman Aug 23 '11 at 14:10
@tadman: As somebody who does quite a bit of FP I agree on the series of transforms things, but I still think that OP's code was readable enough as is, especially since my line is rather long. IMHO DNNX's answer is the sweet spot here. – Michael Kohl Aug 23 '11 at 14:13
The Guard Pattern is a good fit for this case, indeed, though knowing more than one trick is always handy. – tadman Aug 23 '11 at 14:31

score 3 · Accepted Answer · answered Aug 23 '11 at 18:44

Since you're looking for a particular file in each of the directories, just let Dir#[] find them for you, completely eliminating the need to check for a directory. In addition, IO#puts will accept an array, putting each element on a new line. This will get rid of another level of nesting.

File.open('Data/Networks/to_process.txt', 'w') do |out|
  Dir['Data/Networks/*/list.txt'] do |file|
    dir = File.basename(File.dirname(file))
    out.puts File.readlines(file).map { |l| "#{dir}/#{l.split.first}" }
  end
end

score 1 · Answer 4 · answered Aug 23 '11 at 23:00

Reducing the nesting a bit by separating the input from the output:

directories = Dir['Data/Networks/*'].find_all{|f| File.directory?(f)}
output_lines = directories.flat_map do |f|
  output_lines_for_directory = File.open("#{f}/list.txt").map do |line|
    File.basename(f) + "/" + line.split(" ")[0]
  end
end
File.open('Data/Networks/to_process.txt', 'w') do |out|
  out.puts output_lines.join("\n")
end

Too much nesting in ruby?

4 Answers4