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

Re: Duplication vs Intention Revealing code

Expand Messages
  • Anthony Williams
    ... Is it only me and my spell checker that spells 40 forty ? I ll have a go at the exercise later, and post my code. Anthony -- Anthony Williams Software
    Message 1 of 3 , Jun 1, 2005
    • 0 Attachment
      Tony Byrne <yahoogroups@...> writes:

      > Hello Jeff,
      >
      > Wednesday, June 1, 2005, 3:59:14 AM, you wrote:
      > JG> I can go on to bigger numbers, but I see duplication here:
      > JG> - The two "base 10" numbers, used to separate "tens" from "ones".
      > JG> - That "four" and "four-teen" and "four-ty" is duplication.
      >
      > Now that's a new insight for me! It never occurred to me to
      > programmatically handle the appending of 'teen' or 'ty'. Mind you
      > won't 'Fivety' be wrong?

      Is it only me and my spell checker that spells 40 "forty"?

      I'll have a go at the exercise later, and post my code.

      Anthony
      --
      Anthony Williams
      Software Developer
    • Anthony Williams
      ... Code, as promised (C++) follows below. Tony s original duplication is now reduced to the useAnd boolean in the numberAsString function. I thought about
      Message 2 of 3 , Jun 1, 2005
      • 0 Attachment
        Anthony Williams <anthony_w.geo@...> writes:

        > I'll have a go at the exercise later, and post my code.

        Code, as promised (C++) follows below.

        Tony's original duplication is now reduced to the "useAnd" boolean in the
        numberAsString function. I thought about adding it into the MagLabelFinder
        classes, but I didn't feel the need --- if I was to use commas for e.g. one
        thousand, one hundred and two, then I'd probably add a remainderSeparator(int
        remainder) function to the MagLabelFinder, to avoid adding too much logic in
        numberAsString.

        I can see that there is possible duplication between TensLabelFinder and
        SimpleLabelFinder, but I'm not really sure it's enough to warrant doing
        anything about it.

        Anthony
        ==================

        #include "arraysize.hpp"
        #include <string>
        #include "boost/shared_ptr.hpp"

        std::string numberAsString(int i);

        struct MagLabelFinder
        {
        virtual ~MagLabelFinder()
        {}
        virtual int magnitude() const=0;
        virtual std::string label(int magValue) const=0;
        virtual bool canHandle(int value) const=0;
        };

        typedef boost::shared_ptr<MagLabelFinder> MagLabelFinderPtr;

        struct TensLabelFinder:
        MagLabelFinder
        {
        int magnitude() const
        {
        return 10;
        }
        std::string label(int magValue) const
        {
        std::string const tenStrings[]={
        "twenty","thirty","forty","fifty","sixty","seventy","eighty","ninety"
        };

        return tenStrings[magValue-2];
        }
        bool canHandle(int value) const
        {
        return value>10 && value<100;
        }
        };

        struct MinusLabelFinder:
        MagLabelFinder
        {
        int magnitude() const
        {
        return -1;
        }
        std::string label(int magValue) const
        {
        return "minus " + numberAsString(magValue);
        }
        bool canHandle(int value) const
        {
        return value<0;
        }
        };

        struct SimpleLabelFinder:
        MagLabelFinder
        {
        int magnitude() const
        {
        return 1;
        }

        static std::string const strings[];

        bool canHandle(int value) const;

        std::string label(int magValue) const
        {
        return strings[magValue];
        }
        };

        std::string const SimpleLabelFinder::strings[]={
        "zero","one","two","three","four","five","six","seven","eight","nine",
        "ten","eleven","twelve","thirteen","fourteen","fifteen","sixteen","seventeen","eighteen","nineteen"
        };

        bool SimpleLabelFinder::canHandle(int value) const
        {
        return value>=0 && value<arraySize(strings);
        }

        struct BigMagLabelFinder:
        MagLabelFinder
        {
        int const magVal;
        std::string const magLabel;

        BigMagLabelFinder(int magVal_,std::string const& magLabel_):
        magVal(magVal_),magLabel(magLabel_)
        {}

        int magnitude() const
        {
        return magVal;
        }
        std::string label(int magValue) const
        {
        return numberAsString(magValue)+" "+magLabel;
        }
        bool canHandle(int value) const
        {
        return value>=magVal;
        }
        };


        MagLabelFinderPtr findMagnitude(int i)
        {
        MagLabelFinderPtr const magnitudes[]={
        MagLabelFinderPtr(new MinusLabelFinder),
        MagLabelFinderPtr(new SimpleLabelFinder),
        MagLabelFinderPtr(new BigMagLabelFinder(1000000000,"billion")),
        MagLabelFinderPtr(new BigMagLabelFinder(1000000,"million")),
        MagLabelFinderPtr(new BigMagLabelFinder(1000,"thousand")),
        MagLabelFinderPtr(new BigMagLabelFinder(100,"hundred")),
        MagLabelFinderPtr(new TensLabelFinder)
        };

        for(unsigned magIndex=0;magIndex<arraySize(magnitudes);++magIndex)
        {
        if(magnitudes[magIndex]->canHandle(i))
        {
        return magnitudes[magIndex];
        }
        }
        }

        std::string numberAsString(int i)
        {
        MagLabelFinderPtr const magLabelFinder=findMagnitude(i);
        int const magValue=i/magLabelFinder->magnitude();
        int const remainder=i%magLabelFinder->magnitude();

        bool const useAnd=magLabelFinder->magnitude()>10 && remainder<100;
        return magLabelFinder->label(magValue)+(remainder?(useAnd?" and ":" ")+numberAsString(remainder):"");
        }
      • Anthony Williams
        ... Actually, I decided that the duplication between SimpleLabelFinder and TensLabelFinder was worth eliminating, which yielded LookupLabelFinder: struct
        Message 3 of 3 , Jun 1, 2005
        • 0 Attachment
          Anthony Williams <anthony_w.geo@...> writes:

          > I'll have a go at the exercise later, and post my code.

          Actually, I decided that the duplication between SimpleLabelFinder and
          TensLabelFinder was worth eliminating, which yielded LookupLabelFinder:

          struct LookupLabelFinder:
          MagLabelFinder
          {
          int const magVal;
          std::string const* strings;
          unsigned const numStrings;

          template<unsigned numStrings_>
          LookupLabelFinder(int magVal_,std::string const (&strings_)[numStrings_]):
          magVal(magVal_),strings(strings_),numStrings(numStrings_)
          {}

          int magnitude() const
          {
          return magVal;
          }
          std::string label(int magValue) const
          {
          return strings[magValue];
          }
          bool canHandle(int value) const
          {
          return value<(magVal*numStrings);
          }
          };

          MagLabelFinderPtr findMagnitude(int i)
          {
          static std::string const simpleStrings[]={
          "zero","one","two","three","four","five","six","seven","eight","nine",
          "ten","eleven","twelve","thirteen","fourteen","fifteen","sixteen","seventeen","eighteen","nineteen"
          };

          static std::string const tenStrings[]={
          "","","twenty","thirty","forty","fifty","sixty","seventy","eighty","ninety"
          };

          MagLabelFinderPtr const magnitudes[]={
          MagLabelFinderPtr(new MinusLabelFinder),
          MagLabelFinderPtr(new LookupLabelFinder(1,simpleStrings)),
          MagLabelFinderPtr(new BigMagLabelFinder(1000000000,"billion")),
          MagLabelFinderPtr(new BigMagLabelFinder(1000000,"million")),
          MagLabelFinderPtr(new BigMagLabelFinder(1000,"thousand")),
          MagLabelFinderPtr(new BigMagLabelFinder(100,"hundred")),
          MagLabelFinderPtr(new LookupLabelFinder(10,tenStrings)),
          };

          for(unsigned magIndex=0;magIndex<arraySize(magnitudes);++magIndex)
          {
          if(magnitudes[magIndex]->canHandle(i))
          {
          return magnitudes[magIndex];
          }
          }
          }
        Your message has been successfully submitted and would be delivered to recipients shortly.