How can I prettify blocks with more than 2 of each block?

Actually, I just have no ideas.

How can I make breaklines to make it more readable?

Can anyone suggest ways on improving this?

module TestingDataLoad
  def get_files_from(search_target)

    Dir[search_target].each do | d |
      testing_files={}
      if File.directory? d
        Dir.glob("#{d}/*json") do |file|
          if File.basename(file,".json") =~ /(\w+)(_\d+-\d+)/
            event_type = $1.to_sym
            if testing_files.has_key? event_type
              testing_files[event_type] << file
            else
              testing_files[event_type] = [file]
            end
          elsif File.basename(file,".json") =~ /(\w+)/
            event_type = $1.to_sym
            testing_files[event_type] = [file]
          end
        end
      end

      return testing_files
    end
  end

A couple of things I noticed:

• Dir[...] and Dir.glob(...) are synonymous, but you're using both forms?

• Speaking of, glob accepts fairly complex patterns, which could help narrow the search

• Skip the testing_files.has_key? check; just use testing_files[event_type] ||= []

• The explicit return makes little sense. You're looping through the globbed dir paths, but your explicit return means that only the first will ever be checked. Maybe there's only supposed to be one path, but that depends on what search_target is, which you don't know.

• Your method will return nil, an empty hash or a populated hash. Limit the number of possible return values.

Mostly though, it seems like the filename-branching is unnecessary. In both cases you add grab the \w+ part of the name, and use it as a hash key for an array. The only difference is that
if the filename has numbers in it, it'll add the file to the array, otherwise it'll replace the array with a new 1-element array.

I'd say treat all files the same, and append them to the array. Don't ever replace the array. As far as I can figure, your method is entirely dependent on the platform/filesystem you're running it on, since glob doesn't dictate the order in which files are returned.

So given 3 files in the following order:

foobar.json
foobar_1-1.json
foobar_1-2.json

your method will return all three. But if the order's reversed, it'll only return "foobar.json". Or, if it's somehow mixed, it might return "foobar.json" and "foobar_1-1.json", but not the 3rd file.

So instead of risking inconsistent behavior, I'd suggest sticking to 1 way of treating the files, regardless of whether they're numbered or not.

def get_files_from(search_target)
  files = {}

  Dir[search_target].each do | d |
    next unless File.directory?(d) # skip if d isn't a dir

    Dir["#{d}/*.json"] do |file|
      event = File.basename(file)[/^\w+/]
      testing_files[event] ||= []
      testing_files[event] << file
    end
  end

  files
end

If you want to be more exacting, you can do this instead

Dir["#{d}/*.json"].select { |file| File.basename(file, '.json') =~ /^\w+(_\d+-\d+)?$/ }.each do |file|
  ...
end

StackExchange Code Review Q#54691, answer score: 4