Prev: Hijacking `super'
Next: Encrypting files in Ruby
From: Aldric Giacomoni on 29 Apr 2010 17:17
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:
http://github.com/Trevoke/SGFParser/blob/2684c78738a0cc8cc8de0ad73f6a96efa7aa1101/lib/sgf/parser/tree_parse.rb

This is the new one:
http://github.com/Trevoke/SGFParser/blob/45dfc8649cbf021c0472d985f8fe9e837e151579/lib/sgf/parser/tree_parse.rb

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

Thanks in advance.
--
Posted via http://www.ruby-forum.com/.

From: David A. Black on 29 Apr 2010 18:25
Hi Aldric --

On Fri, 30 Apr 2010, Aldric Giacomoni wrote:

> 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:
> http://github.com/Trevoke/SGFParser/blob/2684c78738a0cc8cc8de0ad73f6a96efa7aa1101/lib/sgf/parser/tree_parse.rb
>
> This is the new one:
> http://github.com/Trevoke/SGFParser/blob/45dfc8649cbf021c0472d985f8fe9e837e151579/lib/sgf/parser/tree_parse.rb
>
> 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 :-) 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

 | 
Pages: 1
Prev: Hijacking `super'
Next: Encrypting files in Ruby