Code refactoring: request for comment

Hi all,
I’ve just finished doing some pretty heavy refactoring on a method. I’d
like to know if anyone can offer any suggestions or feedback.

This is the old one:

This is the new one:

In short, I am parsing an SGF file.
The basic important characters which delimit the nodes are (, ), ;, [
and ].

Thanks in advance.

Hi Aldric –

On Fri, 30 Apr 2010, Aldric G. wrote:

In short, I am parsing an SGF file.
The basic important characters which delimit the nodes are (, ), ;, [
and ].

In looking it over, the one place where it was hard to see what was
going on was:

       when '['
         get_property
         @content[@identity] ||= ""
         @content[@identity] << @property
         @identity = ""

It’s not clear that get_property does anything to @property. I mean,
it’s clear once you look at get_property, but not from the calling
context.

Could you do this instead…

       when '['
         @content[@identity] ||= ""
         @content[@identity] << get_property
         @identity = ""

and then retool get_property to use a temporary buffer instead of
@property? If so, you could probably get rid of @property entirely.

Now, just for fun, and because I’m a bit of a compulsive refactorer,
I’ve created this: http://gist.github.com/384362 which probably uses
totally wrong method names but which paints a pretty good picture of
what would happen if you wanted to push all the busy work out of the
case statement entirely and let the instance variables and so on talk
to themselves elsewhere. (You’ll see what I mean :slight_smile: I don’t know the
SGF format myself, and haven’t subjected the code to any reality
checks beyond making sure your specs pass.

David


David A. Black, Senior Developer, Cyrus Innovation Inc.

THE Ruby training with Black/Brown/McAnally
COMPLEAT Coming to Chicago area, June 18-19, 2010!
RUBYIST http://www.compleatrubyist.com

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs