On Fri, Mar 19, 2010 at 10:31 AM, Robert Klemme
<shortcutter / googlemail.com>wrote:

> 2010/3/19 Shiny Hydra <slotriof / guerrillamailblock.com>:
> >> 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
> >
> > Thank you very much for writing up a sample of how I could begin making
> > a script to manipulate this data. This provides a fantastic spot to jump
> > in and begin building this application!
>
> See also http://groups.google.de/group/comp.lang.ruby/msg/c53f394410a6cff0
> which did not (yet) make it into the mailing list.
>
> Kind regards
>
> robert
>
> --
> remember.guy do |as, often| as.you_can - without end
> http://blog.rubybestpractices.com/
>
>
Hi, Robert

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

That is fine, I appreciate what you did take the time to go through :)

> - 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.

That name would probably make sense. I'm not sure what you mean by Class
comments, though.


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

Well, each line must match the line length, or when we try to pull specific
attributes from that record they will not be correct. If each line is the
correct length, then the record is the correct length.


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

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

That makes a lot of sense, thanks for the tip, I default to OO first,
because that is what I am most familiar with, so structs don't come quickly
to mind. Though I have read your article probably three times
http://blog.rubybestpractices.com/posts/rklemme/017-Struct.html it usually
only comes to mind it when my structure is very dynamic, ie OpenStruct
> - Your MailRecord is a good abstraction of the file storage.

Thank you :)

> > - 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.

> - 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.

I understand your points here, but I am having difficulty thinking of a way
to implement this. Perhaps the record should have a reference to the
MailRecord (or MailFile, as you suggest), and then tell the file to write
itself? But the position thing still seems to be an issue.

Maybe the problem is that I am considering it to be almost an array of files
based on position in the file, but I should remove index/offset from
consideration and instead consider it as a set, where ordering is arbitrary
and can be altered as necessary to accommodate encapsulated logic and data
integrity.


I'm not really sure what a proper approach would look like here.

Anyway, thanks for taking the time to look and comment :)