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

RE: [XP] Candidate for "Replace Conditional With Polymorphism"? ( long)

Expand Messages
  • Blum, Robert
    Aah - I get it. I still don t think it s a candidate for replace with polymorphism Let me try a different approach (I m trying to learn the refactoring stuff
    Message 1 of 1 , Feb 28, 2001
    • 0 Attachment
      Aah - I get it. I still don't think it's a candidate for 'replace with
      polymorphism'

      Let me try a different approach (I'm trying to learn the refactoring stuff
      too - maybe we can see it as sort of a 'paired excercise'?)
      I hope the gurus have time enough to point out the problems in my
      approach...

      First attempt:

      class Referee
      {
      ...

      bool canLockPairOfBoxes()
      {
      if(getBox(m_iPips1).isPermanent()) {
      return false;
      }

      if(getBox(m_iPips2).isPermanent()) {
      return false;
      }

      return true;
      }

      bool canLockSingleBox()
      {
      if(getBox(m_iPips1+m_iPips2).isPermanent()) {
      return false;
      }

      return true;
      }

      void lockSingleBox()
      {
      if(!canLockSingleBox()) {
      return;
      }

      getBox(m_iPips1+m_iPips2).lock();
      }

      void lockPairOfBoxes()
      {
      if(!canLockPairOfBoxes()) {
      return;
      }

      getBox(m_iPips1).lock();
      getBox(m_iPips2).lock();
      }
      }

      Second attempt:
      The above would have to introduce knowledge of the rules in the U/I to
      actually limit the player to valid choices. That's not good.
      So let's add two methods to referee:

      first, to determine if a players choice is valid:

      bool isValidChoice(int i)
      {
      if(i==m_iPips1) {
      return true;
      }
      if(i==m_iPips2) {
      return true;
      }
      if(i==m_iPips1+m_iPips2) {
      return true;
      }
      }

      second, we create a method getLockCandidates() that will return a list of
      locked boxes, depending on the choice

      int[] getLockCandidates(int choice)
      {
      int[] result;

      if(!isValidChoice(choice)) {
      return null;
      }


      if(i==m_iPips1||i==m_iPips2) {
      result=new int[2];
      result[0]=m_iPips1;
      result[1]=m_iPips2;
      } else {
      result=new int[1];
      result[0]=i;
      }

      return result;
      }

      Third attempt:
      By now, we have a lot of duplication in our code. Let's introduce a
      Hashtable for valid choices and the boxes they lock.

      class Referee {
      ...
      Hashtable choiceToBoxMapping;

      void setDiceRoll(int pips1,int pips2)
      {
      choiceToBoxMapping=new Hashtable();
      int [] boxes=new int[2];
      boxes[0]=pips1;
      boxes[1]=pips2;

      choiceToBoxMapping.put(new Integer(pips1),boxes);
      choiceToBoxMapping.put(new Integer(pips2),boxes);

      boxes=new int[1];
      boxes[0]=pips1+pips2;
      choiceToBoxMapping.put(new Integer(pips1+pips2),boxes);
      }

      getLockCandidates becomes
      int[] getLockCandidates(int choice) {
      return choiceToBoxMapping.get(choice);
      }

      isValidChoice now becomes much simpler - just check if there's a mapping
      bool isValidChoice(int choice)
      {
      int boxes[]=getLockCandidates(new Integer(choice));

      if(boxes==null) {
      return false;
      }

      return true;
      }
      The two canLock functions now become one

      bool canLock(int choice)
      {
      if(!isValidChoice(choice)) {
      return false;
      }

      int boxes[]=getLockCandidates(choice);

      for(int i=i;i<boxes.length;i++) {
      if(getBox(boxes[i]).isPermanent()) {
      return false;
      }
      }

      return true;
      }

      The same for the two lock methods

      void lock(int choice)
      {
      if(!canLock(choice)) {
      return;
      }

      int boxes[]=getLockCandidates(choice);
      for(int i=i;i<boxes.length;i++) {
      getBox(boxes[i]).lock();
      }
      }

      If I were to continue further, I'd probably make 'choice' an object - I pass
      it to every function I call, so those functions would be choice members.
      Refereee would be able to create a choice. At this point, I might be willing
      to have to objects sharing a common interface - ValidChoice and
      InvalidChoice. Referee would be the factory for those objects.

      OK, that's it - go and put holes into it :-)

      Bye,
      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.
    Your message has been successfully submitted and would be delivered to recipients shortly.