CODEWITHSUNDEEP
rubynested
Steve C
Steve C

Reputation: 33

Too much nesting in ruby?

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!

Upvotes: 3

Views: 402

Answers (4)

Andrew Grimm
Andrew Grimm

Reputation: 81500

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

Upvotes: 1

thorncp
thorncp

Reputation: 3627

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

Upvotes: 3

DNNX
DNNX

Reputation: 6255

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.

Upvotes: 8

Michael Kohl
Michael Kohl

Reputation: 66837

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

Upvotes: 6

Related Questions