On Monday, December 27, 2004, at 2:39:16 AM, aacockburn wrote:
> 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.
Generally I don't know where they are heading either. In future
articles I'll try to be more explicit about what was part of my
cunning plan and what was just the next discovery. Usually its the
latter.
> << 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�
Well, I can't offer anything for your stomach but in code for
sharing and publication, I think one needs to hone pretty closely to
the "standard". Others might disagree, and in the privacy of one's
own home, one can do whatever one wants.
> <<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.
Yes. Code that looks like that is trying to tell us something. It's
not always clear what.
> 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).
Hmm, well, there's the inherent need of the universe to be in
balance. Did you consider that? :)
> 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):
I hear ya on the journal thing. It feels like it about quadruples
the time it takes me to do anything.
> 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 ]
Well, I think you need to get over the inject:into: thing, but I'll
leave it to you and your shrink to put that into priority order with
the other stuff that you're working on. (Or that we hope you're
working on, anyway.)
That said, summedWith: isn't a bad idea. Did you implement it with
inject:into:? :)
I believe that the major reason why inject:into: might be thought
"better" than summedWith: is that summedWith: assumes more about the
objects in the collection, namely that they respond to plus, and
that they can add themselves to zero. It has the advantage, of
course, of being more communicative to folks with mysterious and
unexplained reactions to inject:into:. There are always these
tradeoffs.
> 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
I think I'd like firstRoll + secondRoll + bonusRoll better ...
> 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
The above is on the hairy edge of too complex for my taste. I don't
see how to simplify it, but my smeller would be asking me to.
> Frame>>fromRolls: rolls at: frameStart
> roll1 := rolls at: frameStart.
> roll2 := rolls at: frameStart + 1.
> self isSpareOrStrike
> ifTrue: [ bonusRoll := rolls at: frameStart + 2 ].
> ^ self
Here, I'd probably set bonusRoll to zero explicitly in an ifFalse:,
which would save an init that must be somewhere else, though I don't
see it. Setting it both ways here might be a bit more explicit.
> Frame>>rollsUsed
> ^ self isOpenOrSpare
> ifTrue: [ 2 ]
> ifFalse: [ 1 ]
Yes, but why not
rollsUsed
^self isStrike
ifTrue: [ 1 ]
ifFalse: [ 2 ]
> 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)
Yes, it does look much more Smalltalkish. I would agree that the
inits and refs to frame are big concerns, and would add the general
complexity of that frame-building method. But the solutions we've
seen so far, while more compact and perhaps looking more
Smalltalkish, were a bit tricky to understand. Again, those
tradeoffs.
> Summarizing:
> Frame >> has 3 inst vars: roll1, roll2, bonusRoll
> the accessors, of course
> isOpen, isSpare, isStrike, isOpenOrSpare, isSpareOrStrike
> rollsUsed
> fromRolls:at:
> score
I'd think that one could cut down the is's. It seems to me that
there are only two interesting cases:
1. A strike frame increments by 1 and all others by 2.
2. An open frame adds up two rolls, and all others 3.
HOWEVER, I notice that some people get freaky when you merge the
strike and spare as implied by 2. Still, I think we could probably
get by without 5 booleans, which do suggest a space of 32
possibilities, when we've only really got two or three.
> 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
Yes, that does seem rather nice ...yet the smallness of it might go
to your next comment. When one of the extracted classes is that
simple, it might argue that it's too soon to split.
> 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.
I would not, but it's probably a matter of taste (my good taste vs
your execrable, was what I had in mind, but some might reverse the
adjectives), and of reading and writing style.
I'm not sure I can pull this next idea out, but here's a draft:
Part of the skill in writing and reading Smalltalk is /not/ to
look at all the classes, instance variables, and methods. One
works in a way where one never really tries to grasp the 12
methods all at once. Instead, something like
self isStrike ifTrue: [^self sumThreeBalls].
self isSpare ifTrue: [^self sumThreeBalls].
^self sumTwoBalls
is just taken at face value, in the sense that we say, OK, if
isStrike and isSpare work, and if sumThreeBalls does what it
sounds like and sumTwoBalls does what it sounds like then I see
that this will work.
Then we might subsequently explore those lower levels, to see how
/they/ work, but we are no longer concerned with how the whole
thing works.
In contrast, looking at the big method we originally started with,
we sort of have to keep all the ideas in our head at the same
time. Smalltalk makes it easier to create those small separate
abstractions, but it is a bit of a different mindset to work that
way.
> On the other hand, it was a good feeling to watch the
> responsibilities get pushed into Frame.
Isn't it? It's kind of occupying, yet somehow mindless. A spectator
sport of watching oneself. I find it a fascinating activity.
Ron Jeffries
www.XProgramming.com
The main reason that testing at the end of a development cycle finds
problems is not that problems were put in near the end, it is that
testing was put off until then.
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/
