views:

91

answers:

3

I have a parser that reads files. Inside a file, you can declare a filename and the parser will go and read that one, then when it is done, pick up right where it left off and continue. This can happen as many levels deep as you want.

Sounds pretty easy so far. All I want to do is print out the file names and line numbers.

I have a class called FileReader that looks like this:

class FileReader
    attr_accessor :filename, :lineNumber
    def initialize(filename)
        @filename = filename
        @lineNumber = 0
    end
    def readFile()
        # pseudocode to make this easy
         open @filename
         while (lines)
             @lineNumber = @lineNumber + 1
             if(line startsWith ('File:'))
                 FileReader.new(line).readFile()
             end
             puts 'read ' + @filename + ' at ' + @lineNumber.to_s()
         end
         puts 'EOF'
    end
end

Simple enough. So lets say I have a file that refers other files like this. File1->File2->File3. This is what it looks like:

read File1 at 1
read File1 at 2
read File1 at 3
read File2 at 1
read File2 at 2
read File2 at 3
read File2 at 4
read File3 at 1
read File3 at 2
read File3 at 3
read File3 at 4
read File3 at 5
EOF
read File3 at 5
read File3 at 6
read File3 at 7
read File3 at 8
EOF
read File2 at 4
read File2 at 5
read File2 at 6
read File2 at 7
read File2 at 8
read File2 at 9
read File2 at 10
read File2 at 11

And that doesnt make any sense to me.

File 1 has 11 lines
File 2 has 8 lines
File 3 has 4 lines

I would assume creating a new object would have its own scope that doesn't affect a parent object.

A: 

I agree that something in your rewriting code has obfuscated the problem. Yes, those instance variables should be local to the instance.

Watch out for things where a block of code or conditional may be returning a value and assigning it to the instance variable... for example, if your open statement uses the next block and returns the filename somehow... @filename = open(line) {}

I say this because the filename obviously didn't change back after the EOF

DGM
+3  A: 
class FileReader
  def initialize(filename)
    @filename = filename
  end

  def read_file
    File.readlines(@filename).map.with_index {|l, i|
      next "read #{@filename} at #{i}" unless l.start_with?('File:')
      FileReader.new(l.gsub('File:', '').chomp).read_file
    }.join("\n") << "\nEOF"
  end
end

puts FileReader.new('File1').read_file

or just

def File.read_recursively(filename)
  readlines(filename).map.with_index {|l, i|
    next "read #{filename} at #{i}" unless l.start_with?('File:')
    read_recursively(l.gsub('File:', '').chomp)
  }.join("\n") << "\nEOF"
end

puts File.read_recursively('File1')
Jörg W Mittag
Your answer is much cleaner than mine and a much better example of Ruby. I should have taken a few moments to refactor.
Mike Bethany
After playing around with some file IO I would strongly recommend against just using readlines. It does some really funky stuff when I tried it without the File in front of it. Like it was looking at the last file I tested for existence using the ARGV array instead of the string I passed it. Very weird. Put "File" in front of it and it works great.
Mike Bethany
+2  A: 

This is what I came up with. It's not pretty but I tried to stay as close to your code as possible while Ruby-fying it too.

file_reader.rb

#!/usr/bin/env ruby
class FileReader

  attr_accessor :filename, :lineNumber

  def initialize(filename)
      @filename = filename
      @lineNumber = 0
  end

  def read_file
    File.open(@filename,'r') do |file|
      while (line = file.gets)
        line.strip!
        @lineNumber += 1
        if line.match(/^File/)
          FileReader.new(line).read_file()
        end
        puts "read #{@filename} at #{@lineNumber} : Line = #{line}"
      end
    end
    puts 'EOF'
  end
end

fr = FileReader.new("File1")
fr.read_file

And the File1, File2, and File3 looking like:

Line 1
Line 2
Line 3
File2 
Line 5
Line 6
Line 7
Line 8
Line 9
Line 10
Line 11

Output:

read File1 at 1 : Line = Line 1
read File1 at 2 : Line = Line 2
read File1 at 3 : Line = Line 3
read File2 at 1 : Line = Line 1
read File2 at 2 : Line = Line 2
read File2 at 3 : Line = Line 3
read File2 at 4 : Line = Line 4
read File3 at 1 : Line = Line 1
read File3 at 2 : Line = Line 2
read File3 at 3 : Line = Line 3
read File3 at 4 : Line = Line 4
EOF
read File2 at 5 : Line = File3
read File2 at 6 : Line = Line 6
read File2 at 7 : Line = Line 7
read File2 at 8 : Line = Line 8
EOF
read File1 at 4 : Line = File2
read File1 at 5 : Line = Line 5
read File1 at 6 : Line = Line 6
read File1 at 7 : Line = Line 7
read File1 at 8 : Line = Line 8
read File1 at 9 : Line = Line 9
read File1 at 10 : Line = Line 10
read File1 at 11 : Line = Line 11
EOF

To reiterate we really have to see your code to know where the problems is.

I understand you thinking it has something to do with variable scoping so the question makes sense to me.

For the other people. Please be a little more kind to the novices trying to learn. This is supposed to be a place for helping. Thank you. </soapbox>

Mike Bethany