Hi, Ron,
That refactoring was indeed interesting to watch, since I
couldn't tell where it was heading, and it also sent me down
a different track.
<< The parens aren't necessary, and I know Smalltalk gurus who
will dis you for using unnecessary parens. >>
Ah, well, my greater concern is that /I/ need the parens to make it
easy for my eyes to parse. Probably another year or so in the IDE and
my eyes would adjust. In the meantime, I'm happy to
throw a few extra parens in ...
<< spare := (roll1 < 10) and: [roll1 + roll2 = 10]. >>
Yes, I know this is the proper Smalltalk, and frankly, it makes my
stomach turn (along with "inject:into"). However, I found a different
way to get rid of (isolate) each of these � once you got me going on
the refactoring; which I took in a different direction�
<<If I have any concerns, they are with the instance variables:
Smalltalk defineClass: #Game2
instanceVariableNames: 'rolls score frameStart '
>>
Yes, here is where I parted company. Two of those instance varsexist
to support the inside of a loop inside a method, at which point
I felt it was time to introduce the Frame abstraction � because
(a) much of what was going into the Game's extra methods ought to be
properties of a Frame (e.g. "frameScore" as a method should really be
the 'score' method of Frame), and
(b) two of those inst vars are there because they are being used
inside a loop in a method, really just because you don't like having
those declarations inside the method. I'm not fond of exporting a
loop value all the way out to the class, and would rather just
declare it in the loop or method.
(c) once we introduce Frame, then the game can store frames instead
of rolls, which makes the itch in my back ease up.
But you are right, my former version didn't have the Smalltalk
'feel' (not that it needed it, since there was/is no second
user story to drive it any further in any direction).
I went back to work to get some of that Smalltalk feeling into it.
Not having written a journal, here's the gist (I went through 3 or 4
refactorings to get here):
Personally, I don't think the 'score' method of Game should be any
more complicated than this:
Game>>score
^ frames summedWith: [ :frame | frame score ]
that's because I detest 'inject:into' especially when it is doing
nothing more than summing (cf previous whine). So I added
the 'summedWith:' method to my image ;-). Using 'inject:into' it
looked like:
Game>>score
^ frames inject: 0
into: [ :runningScore :frame |
runningScore + frame score ]
The Frame class is pretty trivial. I didn't mess around with
subclasses. 'score' is of course nothing more than
Frame>> score
^ roll1 + roll2 + bonusRoll
The remaining and reduced complexity got pushed into
Game>>framesFromRolls: rolls
| frameStart |
frames := OrderedCollection new.
frameStart := 1.
10 timesRepeat: [ | frame |
frame := Frame new fromRolls: rolls at: frameStart.
frames addLast: frame.
frameStart := frameStart + frame rollsUsed.
].
^frames
Frame>>fromRolls: rolls at: frameStart
roll1 := rolls at: frameStart.
roll2 := rolls at: frameStart + 1.
self isSpareOrStrike
ifTrue: [ bonusRoll := rolls at: frameStart + 2 ].
^ self
Frame>>rollsUsed
^ self isOpenOrSpare
ifTrue: [ 2 ]
ifFalse: [ 1 ]
and that, I'll contend, does look Smalltalkish (well, enough for
tonight� I can't see how to dump the two initializations or merge
the two refrences to 'frame' ...). The Frame class does frame-y
things, the Game class does game stuff, and there is full separation
between breaking the rolls into frames and scoring (one of the
worries you expressed earlier)
Summarizing:
Frame >> has 3 inst vars: roll1, roll2, bonusRoll
the accessors, of course
isOpen, isSpare, isStrike, isOpenOrSpare, isSpareOrStrike
rollsUsed
fromRolls:at:
score
in the end, Game only has the one instance var, 'frames' (which seems
reasonable, since a game consists of frames, possibly even more than
it consists of rolls), and only two methods:
Game>> has 1 inst var: frames
framesFromRolls:
score
Now, was all that worth it? Well, on the one hand, it is more
Smalltalkish. On the other hand, we went from (1 class, 1 inst var, 1
method) to (2 classes, 4 inst vars, 12 methods). I'd stay with the
single 15-line method until there was call to add complexity.
On the other hand, it was a good feeling to watch the
responsibilities get pushed into Frame.
Alistair
To Post a message, send it to: [EMAIL PROTECTED]
To Unsubscribe, send a blank message to: [EMAIL PROTECTED]
ad-free courtesy of objectmentor.com
Yahoo! Groups Links
<*> To visit your group on the web, go to:
http://groups.yahoo.com/group/extremeprogramming/
<*> To unsubscribe from this group, send an email to:
[EMAIL PROTECTED]
<*> Your use of Yahoo! Groups is subject to:
http://docs.yahoo.com/info/terms/
