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.