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

Re: Candidate for "Replace Conditional With Polymorphism"?

Expand Messages
  • azami@speakeasy.net
    Hmmm, I feel like the locking rules do not belong in the method that does the lock. I d be inclined to go right down the line hiding rule decisions in other
    Message 1 of 4 , Mar 1, 2001
    • 0 Attachment
      Hmmm, I feel like the locking "rules" do not belong in the method that
      does the lock. I'd be inclined to go right down the line hiding rule
      decisions in other methods.

      class Referee
      {
      ...
      void Lock (int i)
      {
      if (isLockable(i))
      getBox(i).lock();
      }

      boolean isLockable(int i)
      {
      // we can never re-lock a box
      if (isLockedForGood(i))
      return false;

      // if it's the sum, we don't need to check any other boxes
      if (i == firstRoll || i == secondRoll)
      return true;

      // i must be part of a pair of boxes - it's only lockable if
      // its partner is not locked permanently
      if (isLockedForGood(firstRoll) || isLockedForGood(secondRoll))
      return false;

      // it's one of a pair, neither of which is locked for good!
      return true;
      }

      ...
      }

      Notes:
      1 - Should Lock() return a value indicating whether it did anything?
      Should the caller know it might not lock? If so, rename it to reflect
      that (LockIfLockable()).
      2 - What's the "current turn" stuff? I've eliminated that on the
      assumption that isLockedForGood() will not return true until the lock
      is "committed" - in which case, it's a new turn, right?
      3 - Someone will complain about the multitude of returns from
      isLockable(). My only defense is that to my eyes the code very well
      expresses the intent of method.
      4 - What happens when you roll doubles?
      5 - My main goal has been to capture the rules in a place where they
      are easily modified. As you've indicated there are different rules
      people play by, this seems like prime candidate for future change,
      such as the sum of the boxes you lock must be the sum of the pips:

      boolean isLockable(int i)
      {
      Vector candidateSet = getUnlockedBoxes();
      return isLockableAsPartOfSet(i, candidateSet, unusedPips);
      }

      Vector getUnlockedBoxes()
      {
      //return a Vector filled with the indexes
      //of any boxes not locked for good.
      }

      boolean isLockableAsPartOfSet(int i, Vector candidateSet, int
      pipsLeft)
      {
      //we should not consider boxes not in candidateSet
      if (!candidateSet.contains(i))
      return false;

      //boxes locked for good won't be in set, no test needed

      //if this would use up the pips, it's lockable
      if (i == pipsLeft)
      return true;

      //it may be part of a set - try it to see if it works
      Vector subSet = candidateSet - i;
      //(iterate over subSet)
      {
      if (isLockableAsPartOfSet(curItem, subSet, pipsLeft - i))
      return true;
      }

      // found that there is no way to lock boxes with the
      // remaining pips - therefore, there's no way to
      // lock this one.
      return false;
      }
    Your message has been successfully submitted and would be delivered to recipients shortly.