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/