From: Robert Klemme Date: 2010-04-20T17:40:47+09:00 Subject: Re: Writing a parser 2010/4/20 Martin Hansen : >> 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). > > I admit that you implementation will be more robust and more efficient. > However, this parser is meant to be run in a *NIX command line > environment so I can connect scripts with pipes (I should perhaps have > stated this in the "specs"). Well, there are also pipes on Windows. And even if your program is run on a POSIX system the file might have been created on a Windows box. It does not matter that much whether you read from a pipe or file. If you create something that only needs #each_line like I did you can pass a wide range of sources - including ARGF which will happily read from stdin if you do not provide file name arguments to your script. > Actually, I have some more text on this > whole affair: > > www.biopieces.org > > and > > http://code.google.com/p/biopieces/wiki/Introduction#Getting_started > > >> >> b) Embedding that in code that follows usual Ruby iterator patterns >> and integrates seamless with Enumerable. >> >>> �def each >>> � �loop { yield self.next } >>> �end >>> end >> >> This implementation has flaws: >> >> 1. The loop does not terminate properly. > > > But I think it does?!? Did you try it out? > From the Ruby book: Which book? > "External iterators are quite simple to use: just call next each time > you want another element. When there are no more elements left, next > will raise a StopIteration excep- tion. This may seem unusual—an > exception is raised for an expected termination condition rather than an > unexpected and exceptional event. (StopIteration is a de- scendant of > StandardError and IndexError; note that it is one of the only exception > classes that does not have the word “error” in its name.) Ruby follows > Python in this external iteration technique. By treating loop > termination as an exception, it makes your looping logic extremely > simple; there is no need to check the return value of next for a special > end-of-iteration value, and there is no need to call some kind of next? > predicate before calling next." Frankly, I dissent that logic. First, it uses exceptions for *regular* control flow. This is bad in itself (I am pretty sure you'll find plenty of articles on the web about this topic if you look for it). Then, it does not even follow conventions implemented in the Ruby standard library (for example, IO#gets returns nil on EOF). > Not that I have been able to get the syntax right and make this work ... > > >> 2. It does not return "self" which is convention for #each >> implementations. > > I have heard of this convention before, but I seem to have missed it in > the Ruby book. I shall see if I can find it. > >> 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. :-) > > I was going to forget about the "silly" class Record and stick to hashes > of strings. So modifying the behavior of each to something like this: > > include 'my_record' > > File.open('my_file', 'r').each do |record| >  puts record.class # => Hash > end > > Or is this completely nuts? It does not work. File.open returns a File (or IO) instance. Invoking #each on this will fill "record" with lines - not records. Your line "record.class" will print "String" and not "Hash". But I see that basically you want what I provided: a mechanism to iterate records with #each. At the moment I am not sure where we stand. You seem to insist that something with my implementation is wrong and external iteration is better - maybe because you have read about it in some book. Realistically the need for external iteration rarely surfaces, mostly it is if you need to iterate two or more collections in lock step. In all other cases using #each and like mechanisms is superior because it is less error prone. Cheers robert -- remember.guy do |as, often| as.you_can - without end http://blog.rubybestpractices.com/