2010/4/19 Martin Hansen <mail / maasha.dk>:
> @Robert,
>
> I was wondering about the whole approach,

Erm, that's a rather broad statement.  My approach had two parts:

a) Implementation of parsing which I changed from using gets() with a
delimiter because I found that approach too fragile (think of files on
different platforms with different line delimiters, in this case the
defined delimiter does not always work).

b) Embedding that in code that follows usual Ruby iterator patterns
and integrates seamless with Enumerable.

> keeping in mind what Ryan
> suggested (drop class Record which apparently is silly, and loose
> dangling IO stuff). It strikes me that the simplest way to solve this is
> to create an external iterator as a mixin (O'Reilly, Ruby Programming
> Language, p 138). This is pretty much what you already suggested,
> without the foreach method, which I don't need.
>
> module Record
>   
>
>   
>  >  
> end

This implementation has flaws:

1. The loop does not terminate properly.

2. It does not return "self" which is convention for #each implementations.

3. Worst: there is a design issue here.  An #each method of a Record
is expected to iterate through items _within_ (or contained in) the
Record.  But what you are actually attempting here is to have
something that during iteration returns records (regardless of their
type).  Since your method #each does not have arguments member
variables are needed to determine where to fetch records from.  But
since you call this module "Record" you have a weird situation that
you need to create something Recordish (i.e. something with record
state) which also has non record state (for example an IO object) to
start returning other records from somewhere else.  This is highly
confusing and hence something I would not let pass if I were doing the
code review on your code. :-)

Kind regards

robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/