Return to Answer

Your two code samples are not exactly equivalent. The second one has nicer formatting (fixed-width numbering) and attempts to parameterize the glob pattern (but fails in that regard — full_directory_navigator should accept a pattern parameter and pass it to both Dir.glob and list_directory_entries).

I'm not convinced that splitting up the select_file function into six functions makes it any better. It's not any more readable or extensible than the original. The parameter passing just gets in the way. (If you had defined a class, there might be some advantage, in that a subclass could customize the behaviour by overriding one of the methods.)

With more effective string formatting, the code is simple enough to fit in one function.

• To repeat a character, use string multiplication.
• The ternary conditional could just be an if-qualified expression, since one of the outcomes is nil.
• To make a fixed-width column of numbers, use '%-8d' % number.

def select_file(pattern = '*')
 entries = Dir.glob(pattern, File::FNM_DOTMATCH)
 # Header
 puts '-' * 79,
 "Listing entries in #{File.basename(Dir.pwd)}#{
 " matching \"#{pattern}\"" if pattern != '*'
 }",
 '',
 '%-8s %s' % ['Index', 'Entry'],
 '-' * 79
 # Listing
 entries.each_with_index { |entry, i| puts '%-8d - %s' % [i + 1, entry] }
 # Prompt
 print 'Select File/Folder: '
 entries[gets.to_i - 1]
end
selection = select_file()
puts "Selected: #{selection}"

Note that you haven't done any validation on the input. If the input doesn't look like a number, then gets.to_i - 1 becomes 0 - 1, which refers to the last item in the array.

Your two code samples are not exactly equivalent. The second one has nicer formatting (fixed-width numbering) and attempts to parameterize the glob pattern (but fails in that regard — full_directory_navigator should accept a pattern parameter and pass it to both Dir.glob and list_directory_entries).

I'm not convinced that splitting up the select_file function into six functions makes it any better. It's not any more readable or extensible than the original. The parameter passing just gets in the way. (If you had defined a class, there might be some advantage, in that a subclass could customize the behaviour by overriding one of the methods.)

With more effective string formatting, the code is simple enough to fit in one function.

• To repeat a character, use string multiplication.
• The ternary conditional could just be an if-qualified expression, since one of the outcomes is nil.
• To make a fixed-width column of numbers, use '%-8d' % number.

def select_file(glob='*', glob_flags=File::FNM_DOTMATCH)
 entries = Dir.glob(glob, glob_flags)
 # Header
 puts '-' * 79,
 "Listing entries in #{File.basename(Dir.pwd)}#{
 " matching \"#{glob}\"" if glob != '*'
 }",
 '',
 '%-8s %s' % ['Index', 'Entry'],
 '-' * 79
 # Listing
 entries.each_with_index { |entry, i| puts '%-8d - %s' % [i + 1, entry] }
 # Prompt
 print 'Select File/Folder: '
 entries[gets.to_i - 1]
end
selection = select_file()
puts "Selected: #{selection}"

Note that you haven't done any validation on the input. If the input doesn't look like a number, then gets.to_i - 1 becomes 0 - 1, which refers to the last item in the array.

Your two code samples are not exactly equivalent. The second one has nicer formatting (fixed-width numbering) and attempts to parameterize the glob pattern (but fails in that regard — full_directory_navigator should accept a pattern parameter and pass it to both Dir.glob and list_directory_entries).

I'm not convinced that splitting up the select_file function into six functions makes it any better. It's not any more readable or extensible than the original. The parameter passing just gets in the way. (If you had defined a class, there might be some advantage, in that a subclass could customize the behaviour by overriding one of the methods.)

With more effect string formatting, the code is simple enough to fit in one function.

• To repeat a character, use string multiplication.
• The ternary conditional could just be an if-qualified expression, since one of the outcomes is nil.
• To make a fixed-width column of numbers, use '%-8d' % number.

def select_file(pattern = '*')
 entries = Dir.glob(pattern, File::FNM_DOTMATCH)
 # Header
 puts '-' * 79,
 "Listing entries in #{File.basename(Dir.pwd)}#{
 " matching \"#{pattern}\"" if pattern != '*'
 }",
 '',
 '%-8s %s' % ['Index', 'Entry'],
 '-' * 79
 # Listing
 entries.each_with_index { |entry, i| puts '%-8d - %s' % [i + 1, entry] }
 # Prompt
 print 'Select File/Folder: '
 entries[gets.to_i - 1]
end
selection = select_file()
puts "Selected: #{selection}"

Note that you haven't done any validation on the input. If the input doesn't look like a number, then gets.to_i - 1 becomes 0 - 1, which refers to the last item in the array.

Your two code samples are not exactly equivalent. The second one has nicer formatting (fixed-width numbering) and attempts to parameterize the glob pattern (but fails in that regard — full_directory_navigator should accept a pattern parameter and pass it to both Dir.glob and list_directory_entries).

I'm not convinced that splitting up the select_file function into six functions makes it any better. It's not any more readable or extensible than the original. The parameter passing just gets in the way. (If you had defined a class, there might be some advantage, in that a subclass could customize the behaviour by overriding one of the methods.)

With more effective string formatting, the code is simple enough to fit in one function.

• To repeat a character, use string multiplication.
• The ternary conditional could just be an if-qualified expression, since one of the outcomes is nil.
• To make a fixed-width column of numbers, use '%-8d' % number.

def select_file(pattern = '*')
 entries = Dir.glob(pattern, File::FNM_DOTMATCH)
 # Header
 puts '-' * 79,
 "Listing entries in #{File.basename(Dir.pwd)}#{
 " matching \"#{pattern}\"" if pattern != '*'
 }",
 '',
 '%-8s %s' % ['Index', 'Entry'],
 '-' * 79
 # Listing
 entries.each_with_index { |entry, i| puts '%-8d - %s' % [i + 1, entry] }
 # Prompt
 print 'Select File/Folder: '
 entries[gets.to_i - 1]
end
selection = select_file()
puts "Selected: #{selection}"

Note that you haven't done any validation on the input. If the input doesn't look like a number, then gets.to_i - 1 becomes 0 - 1, which refers to the last item in the array.