From: Robert Klemme <shortcutter@...>
Date: 2010-03-20T21:41:09+09:00
Subject: Re: Replace Text at Specific Positions Across Files

On 03/18/2010 09:27 PM, Josh Cheek wrote:
> This got me excited, my file manipulation isn't very good, so thought I'd
> give it a try. Here is what I got http://gist.github.com/336838

I don't have the time to go completely through your code.  Few remarks 
anyway:

- It is not clear to me why you have MailRecord composed of Records. 
I'd probably rather have picked another name, e.g. MailFile.  Class 
comments would also help.

- Your method #validate is invoked on individual fields but you rather 
want to check the complete record length.

- You can use Struct to easily define Record in less lines

Record = Struct.new :title , :id , :from , :to , :offset

- I would not store the position of the record in the Record instance 
because that way you mix business logic state (record contents) and 
storage related state.  If you have another storage medium your position 
in the Record will be superfluous.

- Your MailRecord is a good abstraction of the file storage.

- I would not use puts with your fixed size records.  Rather I would use 
write and read.  Plus, I'd place those methods in MailRecord and not in 
Record because they are specific to this particular storage medium.

Kind regards

	robert


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