Loading ...
Sorry, an error occurred while loading the content.

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

Expand Messages
  • Kevin Smith
    ... 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
    • Blum, Robert
      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.
      • Larmore, Edward
        ... 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/
        • Larmore, Edward
          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
          • Laurent Bossavit
            ... 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/
              ========================================
            • Robert Sartin
              ... 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.