How do I iterate over a file?

Question

I have a file that looks like this:

[noahc:~/projects/wordsquares] master(+87/-50)* ± cat wordlist/test_word_list.txt card apple joe bird card dart area rear birdbird after boat swim north abbe byes beep

If I open up an irb session, I can do:

2.0.0p247 :001 > file = File.open('./wordlist/test_word_list.txt', 'r') => #<File:./wordlist/test_word_list.txt> 2.0.0p247 :002 > file.readlines => ["card\n", "apple\n", "joe\n", "bird\n", "card\n", "dart\n", "area\n", "rear\n", "birdbird\n", "after\n", "boat\n", "swim\n", "north\n", "abbe\n", "byes\n", "beep \n", "\n"]

But, now I have a class called WordSquareGenerator:

class WordSquareGenerator require 'pry' require 'pry-nav' require './lib/word_list_builder.rb' def initialize(n, file_location) @size_of_square = n @file = load_file(file_location) @word_stem_hash = WordListBuilder.new(n, @file).word_stem_hash @word_list = nil end def word_square_word_list binding.pry @file.each do |w| binding.pry @word_list ? break : solve_for_word_list([word]) end binding.pry end def is_list_valid?(list) (0..@size_of_square - 1).each do |n| (0..@size_of_square - 2).each do |m| return false if list[n][m] != list [m][n] end end @generated_list = list unless @generated_list end def solve_for_word_list(word_array) if word_array.length == 4 @word_list = word_array elsif @word_list else next_words = @word_stem_hash[word_array.map{|w| w[word_array.length]}.join] next_words.each do |word| solve_for_word_list(word_array + [word]) end end end private def load_file(file_location) File.open(file_location, 'r') end end

When I run the word_square_word_list method and hit the first binding.pry, I can do:

2.0.0 (#<WordSquareGenerator:0x007ff3f91c16b0>):0 > @file.readlines => []

and I get an empty array for readlines. How can it be that I'm getting two different results doing the same thing, except one is inside that class and the other isn't?

Don't open a file in a method and leave it open for the rest of the program. Get rid of load_file and use a File.open block that wraps the call to WordListBuilder.new. Also, don't use ternary statements for flow control: @word_list ? break : solve_for_word_list([word]) should be break if @word_list followed by solve_for_word_list([word]). — the Tin Man
– the Tin Man, Commented Dec 8, 2013 at 20:54
@theTinMan Why is the ternary operator so bad for flow control? — Noah Clark
– Noah Clark, Commented Dec 8, 2013 at 21:05
@Noah it's unclear because typically it's for returning one of two values. Readable code > Short code — tckmn
– tckmn, Commented Dec 8, 2013 at 21:07
+1 @Doorknob. Ternary is used (abused) a lot in C and Perl, but it often results in code that hides the logic. Ternary code should be very clean and simple. Reading your example, with a break or a method call, obfuscates what you're really doing, which is breaking if a value is set, or calling another method. In reality, using an if/else would be just as bad. Use a trailing if to break or fall through to the next method call. — the Tin Man
– the Tin Man, Commented Dec 8, 2013 at 21:34

Maurício Linhares · Accepted Answer · 2013-12-08 20:54:13Z

At WordListBuilder you are probably reading the file already and when the action gets back to WordSquareGenerator the file is already at it's end and there's nothing else to read. First, don't do what you're doing now, that is opening the file since this leaks the file handle (you're not closing it anywhere) and someone else reading the handle is causing your code to fail.

Here's how you could do it:

class WordSquareGenerator require 'pry' require 'pry-nav' require './lib/word_list_builder.rb' def initialize(n, file_location) @size_of_square = n @file_location = file_location @word_stem_hash = WordListBuilder.new(n, file_location).word_stem_hash @word_list = nil end def word_square_word_list binding.pry IO.foreach(@file_location) do |word| binding.pry @word_list ? break : solve_for_word_list([word]) end binding.pry end def is_list_valid?(list) (0..@size_of_square - 1).each do |n| (0..@size_of_square - 2).each do |m| return false if list[n][m] != list [m][n] end end @generated_list = list unless @generated_list end def solve_for_word_list(word_array) if word_array.length == 4 @word_list = word_array elsif @word_list else next_words = @word_stem_hash[word_array.map{|w| w[word_array.length]}.join] next_words.each do |word| solve_for_word_list(word_array + [word]) end end end end

And you would also need to update WordListBuilder to do the same. This also has the advantage of closing the file handle automatically for you so you don't have to care about closing it yourself.

tckmn · Accepted Answer · 2013-12-08 20:57:47Z

Try running this before you read the lines:

@file.seek 0

You have probably already read the lines, and you have to seek back to the start of the file before you read them again.

Sample IRB session:

irb(main):001:0> f = File.new 'test.txt' # already existing file => #<File:test.txt> irb(main):002:0> f.readlines => ["this", "is", "a", "test"] irb(main):003:0> f.readlines => [] irb(main):004:0> f.seek 0 => 0 irb(main):005:0> f.readlines => ["this", "is", "a", "test"]

You could even make it a method:

def readlines @file.seek 0 @file.readlines end

Also, make sure to close your file (@file.close)! You should only leave it open for as little time as possible. And you definitely should not leave it open for the whole program. If you don't want to worry about that, just store the lines in a variable instead of keeping the file:

@lines = File.open(file_location) {|f| f.readlines } # you could also use the shortcut form # @lines = File.open(file_location, &:readlines)

@MaurícioLinhares I was assuming that he @file.closed somewhere else in the program - I'll edit my answer
Opening the file in a method, and leaving it open is really bad form, and can take a system to its knees if multiple versions of the app are running and consuming file-handles.
Just to confirm, I am leaving this open. I thought about closing it, but wasn't sure how since it was being used across the two classes. I'll take a look at this and @theTinMan's recommendation and see what I can do.
@Noah Do not leave it open - that will cause bad things. Just store the information you need from it and close immediately.
Learn how to use the "block" form for IO and File commands. Ruby makes it really easy to do the right thing.

Collectives™ on Stack Overflow

How do I iterate over a file?

2 Answers 2

Comments

5 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

5 Comments

Related