Custom grep in Ruby

I finished the Ruby chapter in Seven Languages in Seven Weeks. It tries to make you familiar with the core concepts of several languages rather quickly. Dutifully I did all exercises, but most likely they can be improved to be more Ruby-like.


Write a simple grep that will print the lines of a file having any occurrences of a phrase anywhere in that line (in my example 'bleh'). Include line numbers.

File.open( 'grepFile.txt', 'r' ) do |file|
  lines = []
  line_number = 1
  file.readlines.each do |line|
    lines[line_number] = line
    line_number += 1
  end
  lines.each_index do |i|  
    line = lines[i]
    puts "line #{i.to_s}: #{line}" if line =~ /bleh/
  end
end

File.open( 'grepFile.txt', 'r' ) do |file|
  lines = []
  line_number = 1
  file.readlines.each do |line|
    lines[line_number] = line
    line_number += 1
  end
  lines.each_index do |i|  
    line = lines[i]
    puts "line #{i.to_s}: #{line}" if line =~ /bleh/
  end
end

file.readlines.each should be file.each_line, which will do the same thing (iterate over all the lines) without building an array (and reading the whole file into memory) first. Also instead of keeping track of the line_number manually, you should just use each_with_index. Though actually if you want to append to the end of the array, you actually shouldn't use an index at all, but use << or push instead (if you want the indices to start at 1, you can just insert a nil element first).

That being said if all you want is to read the file into an array, you could just do lines = file.readlines without any loop (or possible [nil] + file.readlines, so that it keeps starting at 1, but I'd rather add 1 to the line number when printing instead of copying the whole array).

Further you can use the methods File.readlines or File.foreach instead of File.open + File#readlines or File.open + File#each_line respectively.

Then when you iterate over lines you should use each_with_index rather than each_index, so you don't have to do lines[i] to get at the value.

As a last note, it's unnecessary to call to_s on an object in #{} - ruby does that automatically.

However the whole approach seems needlessly complicated to me. I don't see why you need to build up an array at all. I'd just iterate over the lines once and output them directly. This in addition to being much shorter also has the advantage, that the program runs in O(1) space (assuming the maximum line length is constant) rather than reading the whole file into memory.

File.foreach('grepFile.txt').each_with_index do |line, i|
  puts "line #{i}: #{line}" if line =~ /bleh/
end

And of course instead of 'grepFile.txt' and /bleh/, you should be using ARGV[1] and Regexp.new(ARGV[0]), as it makes no sense to hardcode these values.

StackExchange Code Review Q#2142, answer score: 8