Project

General

Profile

Actions

Feature #8929

closed

CSV.foreach(filename) without block returns failing Enumerator

Added by martinjos (Martin Sidaway) over 10 years ago. Updated over 10 years ago.

Status:
Closed
Target version:
-
[ruby-core:57283]

Description

CSV.foreach(filename) {|entry| p entry } => works
CSV.foreach(filename).to_a => fails

It gives the following error:

IOError: closed stream
from /home/martin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/csv.rb:1776:in gets' from /home/martin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/csv.rb:1776:in block in shift'
from /home/martin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/csv.rb:1774:in loop' from /home/martin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/csv.rb:1774:in shift'
from /home/martin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/csv.rb:1716:in each' from (irb):7:in each'
from (irb):7:in `to_a'
(...)

Updated by martinjos (Martin Sidaway) over 10 years ago

(I thought it had failed to send this message...)

Updated by martinjos (Martin Sidaway) over 10 years ago

I can see that it is tricky, because a normal enumerator created by calling CSV#each shouldn't necessarily close the file (as the user may want to call #rewind), but when you use CSV.foreach you have no reference to the CSV object, and you don't want the file handle to be left open.

However, I don't see why, in principle, CSV.foreach(filename) as an Enumerator shouldn't be supported. It has clear, well-defined semantics: it just has to close the file automatically after yielding the last entry.

Also, this would be consistent with, amongst other things, File.foreach(filename).

The present behaviour occurs because self.foreach() uses the block form of self.open(), which ensures the csv object is closed after yielding it. It then calls csv.each(&block) inside the open block, which is okay if a block was passed in to self.foreach, but if not, the no-block form of each() simply calls to_enum (from Object). This enumerator then gets passed out of the open block (becoming invalidated in the process), and out of self.foreach.

In other words:

class CSV

def self.foreach(path, options = Hash.new, &block)
open(path, options) do |csv|
csv.each(&block)
end
end

def self.open(*args)
(...)
if block_given?
begin
yield csv
ensure
csv.close
end
else
csv
end
end

def each
if block_given?
while row = shift
yield row
end
else
to_enum
end
end

end

A solution might be to create an alternative version of CSV#each that calls CSV#close after the last entry:

def each_closing
if block_given?
begin
while row = shift
yield row
end
ensure
close
end
else
to_enum(method) # method returns :each_closing
end
end

Then CSV.foreach could be:

def self.foreach(path, options = Hash.new, &block)
open(path, options).each_closing(&block)
end

Updated by jwille (Jens Wille) over 10 years ago

=begin
why not simply return an Enumerator from foreach?

class CSV

def self.foreach(path, options = Hash.new, &block)
  if block_given?
    open(path, options) do |csv|
      csv.each(&block)
    end
  else
    to_enum(__method__, path, options)
  end
end

end

which yields

ary = []; CSV.foreach(filename) {|entry| ary << entry }
ary == CSV.foreach(filename).to_a #=> true
=end

Updated by zzak (zzak _) over 10 years ago

  • Category set to lib
  • Status changed from Open to Feedback
  • Assignee set to JEG2 (James Gray)

Updated by Hanmac (Hans Mackowiak) over 10 years ago

another sample where the difference is shown:

CSV.foreach('test.csv').with_index { |csv,i| p i } #<< fails
CSV.to_enum(:foreach,'test.csv').with_index { |csv,i| p i } # works

i think the code from jwillie would be nearly the best, but i would use this:

return to_enum(method, path, options) unless block_given?

Actions #6

Updated by nobu (Nobuyoshi Nakada) over 10 years ago

  • Tracker changed from Bug to Feature
Actions #7

Updated by nobu (Nobuyoshi Nakada) over 10 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r43135.
Martin, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


csv.rb: foreach enumerator

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0