Sorry, an error occurred while loading the content.

## Re: RE: [XP] Candidate for "Replace Conditional With Polymorphism"?

Expand Messages
• ... I liked Laurent s solution, but would propose a sideways way of looking at it that might be even clearer. I present the following rough pseeeeeeudocode:
Message 1 of 6 , Feb 27, 2001
• 0 Attachment
Larmore, Edward wrote:
>Shut the Box uses two dice and a special wooden playing tray. The tray
>features the numbers 1 - 9 in a row, each of which has a hinged or sliding
>cover. A turn involves repeatedly throwing the dice and shutting or
>covering a number or pair of numbers every throw. The turn ends when no
>numbers can be covered upon the throw of the dice at which point the
>player's total is calculated. The overriding goal is to completely cover
>all numbers or "shut the box" which results in the best possible score of
>zero.

I liked Laurent's solution, but would propose a
sideways way of looking at it that might be even
clearer. I present the following rough
pseeeeeeudocode:

class Move
{
int box1;
int box2; // -1 means unused
}

legalMoveList = GetLegalMoves(die1, die2)
move = GetPlayerMove();
if (legalMoveList.Find(move))
// legal
else
// illegal

Kevin
• Please keep in mind that I don t know how Shut the box works at all, so I m probably missing something. ... Fair enough. However, a few points I did not
Message 2 of 6 , Feb 28, 2001
• 0 Attachment
Please keep in mind that I don't know how 'Shut the box' works at all, so
I'm probably missing something.
>
> In our pilot XP project( a version of the game, "Shut the
> Box" ) we came up
> with the method shown below. The idea is that the Referee
> acts as sort of a,
> well, a referee, to decide whether locking one of the 9 boxes
> is legal,
> based on the roll of the dice and the initial states of the
> boxes. If it's
> legal, the referee locks the box. Otherwise it ignores the request.

Fair enough. However, a few points I did not understand about the code:

a) How does the roll of the dice influence the decision?
b) If the lock is only granted conditionally, shouldn't the method name
reflect that?
c) Would it make sense to factor out the strategy (is a box lockable) so
it's separated from the action?

And on a more detailed level:
a) What is m_iPips1/m_iPips2? Is Pips a term from the game?
b) isPermanent() is a confusing function name for me. Would
'isNotLockable()' or somesuch reflect reality better?
c) I personally find hungarian confusing. It's not really adding any value.
isLockable communicates the data type too, but it's more human-readable
d) I take it m_iPips1 and m_iPips2 refer to two boxes. Would it make more
sense to use objects representing the two boxes instead of an ID?
e) Why the empty else clauses?

> Personally, I find the code confusing, even though I'm one of
> the two who
> wrote it! This also was the location of our first major bug.

Confusing code is always a breeding ground for bugs - nobody really
understands it. (Trust me on this one - I've written more than my share of
confusing code:-)

> Question: Is this a candidate for "Replace Conditional With
> Polymorphism
> (State/Strategy)?"

Hmm. I thought this refactoring only applies if your conditional represents
a type of object. I can see that the boxes referred to by m_iPips1 and
m_iPips2 are special boxes, but what about the one referred to by the sum of
the two?

>Or is that overkill? What is the criteria
> for deciding?
> Simplicity or Ease of Understanding? Or are those terms
> synonymous?

If it is obvious to you that it betters your code, it's a good refactoring.
If you're in doubt, Fowler recommends to not do it. However, since you're
new to refactoring, you probably don't have the gut feelings he has. Try it,
and if you're not absolutely convinced of it, undo it.

>The way
> we did it involved the fewest classes/methods (our definition
> of simplicity
> at the time).

Simplicity should always lead to a desing that communicates clearly - the
number of classes and methods is only a secondary consideration. (At least
for me)

>But now that I've read a bit more of "Refactoring", I'm
> thinking the above mentioned pattern might make it easier to
> understand the
> code, even though there would be more classes.

Aah - the 'Design Patterns Syndrome'. Everybody reading the 'Design
Patterns' book had the unstoppable urge to apply those patterns, no matter
what the cost :-). I think 'Refactoring' causes the same feelings. I'm lucky
we're in crunch mode - I can't follow those urges right now :-)

Hope this was somewhat helpfull,
Robert

--
Disclaimer:

This email may contain confidential information. If you have received this
email in error, please delete it immediately,and inform us of the mistake by
return email.

Please be aware that messages sent over the Internet may not be secure and
should not be seen as forming a legally binding contract unless otherwise
stated.

Also, please note that opinions expressed in this email are those of the
author, and are not necessarily those of Midway Amusement Games LLC.
• ... Sorry. Shut the Box uses two dice and a special wooden playing tray. The tray features the numbers 1 - 9 in a row, each of which has a hinged or sliding
Message 3 of 6 , Feb 28, 2001
• 0 Attachment
> Please keep in mind that I don't know how 'Shut the box' works at all, so
> I'm probably missing something.

Sorry.

Shut the Box uses two dice and a special wooden playing tray. The tray
features the numbers 1 - 9 in a row, each of which has a hinged or sliding
cover. A turn involves repeatedly throwing the dice and shutting or
covering a number or pair of numbers every throw. The turn ends when no
numbers can be covered upon the throw of the dice at which point the
player's total is calculated. The overriding goal is to completely cover
all numbers or "shut the box" which results in the best possible score of
zero.

In the most popular version, you can cover any combination of numbers that
add up to the total on the dice. In the version we're writing, you can only
cover(lock) numbers(boxes) that are exactly equal to the pips on one of the
dice, or their total. For example, if you roll a 3 and a 4, you can only
lock 3 and 4, or 7. Once a box is locked, you can't lock it again. So if the
3 was already locked, your only choice would be to lock the 7. If the 7 was
also locked, your turn ends.

-Ed Larmore
R a t i o n a l
the e-development company

-----Original Message-----
From: Blum, Robert [mailto:rblum@...]
Sent: Wednesday, February 28, 2001 3:21 PM
To: 'extremeprogramming@yahoogroups.com'
Subject: RE: [XP] Candidate for "Replace Conditional With Polymorphism"?

Please keep in mind that I don't know how 'Shut the box' works at all, so
I'm probably missing something.
>
> In our pilot XP project( a version of the game, "Shut the
> Box" ) we came up
> with the method shown below. The idea is that the Referee
> acts as sort of a,
> well, a referee, to decide whether locking one of the 9 boxes
> is legal,
> based on the roll of the dice and the initial states of the
> boxes. If it's
> legal, the referee locks the box. Otherwise it ignores the request.

Fair enough. However, a few points I did not understand about the code:

a) How does the roll of the dice influence the decision?
b) If the lock is only granted conditionally, shouldn't the method name
reflect that?
c) Would it make sense to factor out the strategy (is a box lockable) so
it's separated from the action?

And on a more detailed level:
a) What is m_iPips1/m_iPips2? Is Pips a term from the game?
b) isPermanent() is a confusing function name for me. Would
'isNotLockable()' or somesuch reflect reality better?
c) I personally find hungarian confusing. It's not really adding any value.
isLockable communicates the data type too, but it's more human-readable
d) I take it m_iPips1 and m_iPips2 refer to two boxes. Would it make more
sense to use objects representing the two boxes instead of an ID?
e) Why the empty else clauses?

> Personally, I find the code confusing, even though I'm one of
> the two who
> wrote it! This also was the location of our first major bug.

Confusing code is always a breeding ground for bugs - nobody really
understands it. (Trust me on this one - I've written more than my share of
confusing code:-)

> Question: Is this a candidate for "Replace Conditional With
> Polymorphism
> (State/Strategy)?"

Hmm. I thought this refactoring only applies if your conditional represents
a type of object. I can see that the boxes referred to by m_iPips1 and
m_iPips2 are special boxes, but what about the one referred to by the sum of
the two?

>Or is that overkill? What is the criteria
> for deciding?
> Simplicity or Ease of Understanding? Or are those terms
> synonymous?

If it is obvious to you that it betters your code, it's a good refactoring.
If you're in doubt, Fowler recommends to not do it. However, since you're
new to refactoring, you probably don't have the gut feelings he has. Try it,
and if you're not absolutely convinced of it, undo it.

>The way
> we did it involved the fewest classes/methods (our definition
> of simplicity
> at the time).

Simplicity should always lead to a desing that communicates clearly - the
number of classes and methods is only a secondary consideration. (At least
for me)

>But now that I've read a bit more of "Refactoring", I'm
> thinking the above mentioned pattern might make it easier to
> understand the
> code, even though there would be more classes.

Aah - the 'Design Patterns Syndrome'. Everybody reading the 'Design
Patterns' book had the unstoppable urge to apply those patterns, no matter
what the cost :-). I think 'Refactoring' causes the same feelings. I'm lucky
we're in crunch mode - I can't follow those urges right now :-)

Hope this was somewhat helpfull,
Robert

--
Disclaimer:

This email may contain confidential information. If you have received this
email in error, please delete it immediately,and inform us of the mistake by
return email.

Please be aware that messages sent over the Internet may not be secure and
should not be seen as forming a legally binding contract unless otherwise
stated.

Also, please note that opinions expressed in this email are those of the
author, and are not necessarily those of Midway Amusement Games LLC.

To Post a message, send it to: extremeprogramming@...

To Unsubscribe, send a blank message to:
extremeprogramming-unsubscribe@...

Ad-free courtesy of objectmentor.com

Your use of Yahoo! Groups is subject to http://docs.yahoo.com/info/terms/
• From: Laurent Bossavit [mailto:morendil@micronet.fr] ... There are three possible states of a box: unlocked, locked but not permanent, and locked and
Message 4 of 6 , Feb 28, 2001
• 0 Attachment
From: Laurent Bossavit [mailto:morendil@...]

> One thing that I would still find confusing is Box.isPermanent().
> Does that mean something different from what Box.isLocked() might
> mean ?

There are three possible states of a box: unlocked, locked but not
permanent, and locked and permanent. If it's locked but not permanent, the
player can unlock it. but once he commits (which he must do prior to rolling
again), he can't undo boxes he's locked.

> Rewritten that way, I don't think Replace Conditional With
> Polymorphism is suitable. Referee properly carries out its assigned
> responsibility - applying the logic specified in the game rules for
> whether a given play is valid.

The fact that there are 3 states led me to the idea of using "Replace
Conditional with Polymorphism", where the Box points to an instance of 1 of
3 possible subclasses of a BoxState class. Trying to lock a box would then
delegate the attempt to box's state. Of course that would delegate the
enforcement of the rules to the Box class, and away from Referee, which is
not what we intended.

I think we'll use the ideas that Laurent and others have proposed. I like
them better. They're simple and easy to understand.

Thanks to all!
-Ed Larmore
R a t i o n a l
the e-development company
• ... But I would say your suggestion is a redesign, not a refactoring. Are you sure that you wouldn t break any tests along the way ? There s an interesting
Message 5 of 6 , Mar 1, 2001
• 0 Attachment
> I liked Laurent's solution, but would propose a sideways way of
> looking at it that might be even clearer. I present the following
> rough pseeeeeeudocode:

But I would say your suggestion is a redesign, not a refactoring.
Are you sure that you wouldn't break any tests along the way ?

There's an interesting general question here - is refactoring
*always* preferable to redesigning, even when (setting myself up as
devil's advocate) there is an alternative design clearly superior to
the existing one and you have confidence that you can build the
new design in and still have all the tests pass at the end ?

(I know, it's a loaded question. It's intended that way.)

========================================
The best way to predict the future is to
invent it.
========================================
Laurent Bossavit - Software Architect
>>> laurent.bossavit@... <<<
>>> 06 68 15 11 44 <<<
>> ICQ#39281367 <<
Agence Bless http://www.agencebless.com/
========================================
• ... Are refactoring and redesign mutually exclusive? That hasn t been my experience. For example, the design of the product I am currently working on is
Message 6 of 6 , Mar 1, 2001
• 0 Attachment
--- Laurent Bossavit <laurent.bossavit@...> wrote:
> There's an interesting general question here - is refactoring
> *always* preferable to redesigning, even when (setting myself up as

Are refactoring and redesign mutually exclusive? That hasn't been my
experience. For example, the design of the product I am currently
working on is completely different from what we started with (thank
goodness, yay test first and refactoring!), but we never stopped and
did a total redesign. A couple of the changes were very large. As I've
mentioned here before one major interface addition (Extract Interface
twice from an existing class and replace most uses of the class to uses
of one or both interfaces*) caused signature changes in one method each
for three critical interfaces. It took about two hours between initial
red bar and final green bar on that one and included changes to
something like 90% of the classes in our product.

* - ironically, the class from which the interfaces were extracted no
longer implements them. That got changed in a recent refactoring to
simplify that class.

To refocus, I'll ask specifically: Is there a black/white difference
between refactoring and redesign?

Regards,

Rob

__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail.
http://personal.mail.yahoo.com/
Your message has been successfully submitted and would be delivered to recipients shortly.