5
\$\begingroup\$

My Implementation:

string NumberToEnglish(long n)
{
    StringBuilder builder = new StringBuilder();
    var trillion = 1000000000000;
    var billion = 1000000000;
    var million = 1000000;
    var thousand = 1000;
    var hundred = 100;
    while(n != 0){
        if(n >= trillion){
            builder.AppendFormat("{0} Trillion ",NumberToEnglish(n / trillion));
            n -= (n / trillion) * trillion;
        }
        else if(n >= billion){
            builder.AppendFormat("{0} Billion ",NumberToEnglish(n / billion));
            n -= (n / billion) * billion;
        }
        else if(n >= million){
            builder.AppendFormat("{0} Million ",NumberToEnglish(n / million));
            n -= (n / million) * million;
        }
        else if(n >= thousand){
            builder.AppendFormat("{0} Thousand ",NumberToEnglish(n / thousand));
            n -= (n / thousand) * thousand;
        }
        else if(n >= hundred){
            builder.AppendFormat("{0} Hundred ",NumberToEnglish(n / hundred));
            n -= (n / hundred) * hundred;
        }else if(n >= 20){
            builder.AppendFormat(numerals[(n / 10) * 10] + " ");
            n -= (n / 10) * 10;
        }else{
            builder.Append(numerals[n]);
            n -= n;
        }
    }
    return builder.ToString();
}


Dictionary<long,string> numerals = new Dictionary<long,string>(){
    {   1,          "One"   },
    {   2,          "Two"   },
    {   3,          "Three" },
    {   4,          "Four"},
    {   5,          "Five"},
    {   6,          "Six"}  ,
    {   7,          "Seven"},   
    {   8,          "Eight"},   
    {   9,          "Nine"},
    {   10,         "Ten"   },  
    {   11,         "Eleven"},  
    {   12,         "Twelve"},  
    {   13,         "Thirteen"},
    {   14,         "Fourteen"},
    {   15,         "Fifteen"   },
    {   16,         "Sixteen"   },
    {   17,         "Seventeen"},
    {   18,         "Eighteen"},
    {   19,         "Nineteen"},
    {   20,         "Twenty"    },
    {   30,         "Thirty"    },
    {   40,         "Forty"}    ,
    {   50,         "Fifty" },
    {   60,         "Sixty" },
    {   70,         "Seventy"}, 
    {   80,         "Eighty"}   ,
    {   90,         "Ninety"}   ,
    {   100,        "Hundred"}  ,
    {   1000,       "Thousand"},
    {   1000000,    "Million"   },
    {   1000000000, "Billion" },
    {   1000000000000, "Trillion" }
};

Test Method:

void Main()
{
    Console.WriteLine(NumberToEnglish(2120)); // Two Thousand One Hundred Twenty 
}

How can I improve this?

\$\endgroup\$
  • \$\begingroup\$ You should use const (outside of the method) to hold the trillion , million etc. \$\endgroup\$ – Heslacher Oct 24 '14 at 12:01
  • \$\begingroup\$ I'd improve that by running the number through a battery of regexes instead of an if else tree and modulo the test number with each pass. \$\endgroup\$ – K. Alan Bates Oct 24 '14 at 13:46
6
\$\begingroup\$

I took an object-oriented approach here, and below you can find what I ended up with. Why do I think it's better? Well, basically, because you should need the ability to control how each one of the number parts is processed: whether it's plural, whether it should be changed according to previous (or following) number parts. The latter one does not really apply to english, but it applies to some other languages. If you did all that in one function, you would end up with having a bunch of code, which performs different kinds of tasks (1. retrieve string value 2. apply plural/singular 3. apply gender 4. Apply "forth"/"four" formatting rules). But taking the object oriented approach you now have a standalone part which does thing 1. When you need to think about 2-4, you can integrate other parsers into your code.

// This is what your initial function looks like
public static string NumberToEnglish(long n) {
    StringBuilder builder = new StringBuilder();
    var steps = new INumberPart[] {
        new SimpleNumberPart{Step = 1000000000000}, //trillion
        new SimpleNumberPart{Step = 1000000000}, //billion
        new SimpleNumberPart{Step = 1000000}, //million
        new SimpleNumberPart{Step = 1000}, //thousand
        new SimpleNumberPart{Step = 100}, //hundred
        new DecimalNumberPart(),
        new TeenNumberPart()
    };
    while (n != 0) {
        var firstAvailable = steps.First(s => s.CanBeFormatted(n));
        n = firstAvailable.Format(n, builder);
    }
    return builder.ToString();
}

public interface INumberPart {
    bool CanBeFormatted(long n);
    long Format(long n, StringBuilder s);
}

public class SimpleNumberPart : INumberPart {
    public long Step { get; set; }

    public bool CanBeFormatted(long n) {
        return n >= Step;
    }

    public long Format(long n, StringBuilder s) {
        return ProcessNum(n, Step, s);
    }

    private static long ProcessNum(long n, long step, StringBuilder builder) {
        var stepValue = n / step;
        builder.AppendFormat("{0}", NumberToEnglish(stepValue));
        builder.AppendFormat("{0} ", numerals[step]);
        return n - stepValue * step;
    }
}

public class DecimalNumberPart : INumberPart {
    public bool CanBeFormatted(long n) {
        return n >= 20;
    }

    public long Format(long n, StringBuilder s) {
        s.AppendFormat("{0} ", numerals[(n / 10) * 10]);
        return n - (n/10)*10;
    }
}

public class TeenNumberPart : INumberPart {
    public bool CanBeFormatted(long n) {
        return 0 <= n && n < 20;
    }

    public long Format(long n, StringBuilder s) {
        s.AppendFormat("{0} ", numerals[n]);
        return 0;
    }
}
\$\endgroup\$
  • \$\begingroup\$ You could also make extensions for int or long, .ToHumanReadable() which can be in a separate file. I think using an Interface here is over engineering because technically you would have to define your own int that implements the interface. Just my thought as I have done this doing extensions instead and other devs understand it better. string blah = 144234.ToHumanReadable(); But this is sure a great example of how powerful interfaces are. +1 \$\endgroup\$ – Piotr Kula Oct 24 '14 at 17:09
3
\$\begingroup\$
  • This avoids the problem of rigid object hierarchies (object-oriented code is inherently brittle)
  • The solution can be extended with the addition of additional matchers without affecting its cyclomatic complexity, thus adhering to the OCP.
  • This has the advantage of avoiding recursion.
  • It has the advantage of being able to represent the number 2702508209400 as

    "2 Trillion 7 Hundred 2 Billion 5 Hundred 8 Million 2 Hundred 9 Thousand 4 Hundred"


Currently revising so that the following tests pass:

2782513111456 |"Two Trillion Seven Hundred Eighty Two Billion Five Hundred Thirteen Million One Hundred Eleven Thousand Four Hundred Fifty Six"

9876543212345 |"Nine Trillion Eight Hundred Seventy Six Billion Five Hundred Forty Three Million Two Hundred Twelve Thousand Three Hundred Forty Five"

12345 | "Twelve Thousand Three Hundred Forty Five"

1112345 |" One Million One Hundred Twelve Thousand Three Hundred Forty Five"

17 | "Seventeen"

12 | "Twelve"

2 | "Two"

(Old) I didn't go into decade sequences because of the additional parsing complexity (ten vs twenty vs thirty...etc) but the gist is the same....just slightly more code within the Pass logic and some additional matchers. I also didn't handle string representing the individual base10 digits, but it's the same algorithm.

This is untested (still need to implement the Mod for progressive declension through the stack of parsers) I wrote it all straight from memory, so if you try it, you'll want to debug.


Edit

Note that I'm adding a singular "Teen" regex resolver for turning 13,14,15,16,17,18,19 into String.Format("{0}teen",n); I'll report the current status of the implementation below

Broken

(I will change from "Broken" to "Fixed" when the implementation is finished...but I'm at work. Tinkering with this during my pomodoro breaks)


public class KataSolver
{
   private System.Int64 Number{get;set;}
   private System.Int64 TestNumber{get;set;}
   public List<Matcher> Matchers{get;set;}
   public KataSolver(int numberToLex)
   {
      Number = numberToLex;
      Matchers = new List<Matcher>();
      Matchers.Add(new Matcher(){Regex="[0-9]{13}$", Token="Trillion", Mod=1000000000000});
      Matchers.Add(new Matcher(){Regex="[0-9]{12}$", Token="Hundred", Mod=100000000000});
      Matchers.Add(new Matcher(){Regex="[0-9]{10}$", Token="Billion", Mod=100000000});
      Matchers.Add(new Matcher(){Regex="[0-9]{9}$", Token="Hundred", Mod=10000000});
      Matchers.Add(new Matcher(){Regex="[0-9]{7}$", Token="Million", Mod=1000000});
      Matchers.Add(new Matcher(){Regex="[0-9]{6}$", Token="Hundred", Mod=100000});
      Matchers.Add(new Matcher(){Regex="[0-9]{4}$", Token="Thousand",Mod=1000});
      Matchers.Add(new Matcher(){Regex="[0-9]{3}$", Token="Hundred", Mod=100});
      Matchers.Add(new Matcher(){Regex="^(90)$", Token="Ninety", Mod=90});
      Matchers.Add(new Matcher(){Regex="^(80)$", Token="Eighty", Mod=80});
      Matchers.Add(new Matcher(){Regex="^(70)$", Token="Seventy", Mod=70});
      Matchers.Add(new Matcher(){Regex="^(60)$", Token="Sixty", Mod=60});
      Matchers.Add(new Matcher(){Regex="^(50)$", Token="Fifty", Mod=50});
      Matchers.Add(new Matcher(){Regex="^(40)$", Token="Forty", Mod=40});
      Matchers.Add(new Matcher(){Regex="^(30)$", Token="Thirty", Mod=30});
      Matchers.Add(new Matcher(){Regex="^(20)$", Token="Twenty", Mod=20});
      Matchers.Add(new Matcher(){Regex="^(13|14|15|16|17|18|19)$",Token="{0}teen",Mod=10});
      Matchers.Add(new Matcher(){Regex="^(12)$", Token="Twelve", Mod=12});
      Matchers.Add(new Matcher(){Regex="^(11)$", Token="Eleven", Mod=11});
      Matchers.Add(new Matcher(){Regex="^(10)$", Token="Ten", Mod=10});
      Matchers.Add(new Matcher(){Regex="^(9)$", Token="Nine",Mod=9});
      Matchers.Add(new Matcher(){Regex="^(8)$", Token="Eight",Mod=8});
      Matchers.Add(new Matcher(){Regex="^(7)$", Token="Seven",Mod=7});
      Matchers.Add(new Matcher(){Regex="^(6)$", Token="Six",Mod=6});
      Matchers.Add(new Matcher(){Regex="^(5)$", Token="Five",Mod=5, AltToken="Fif"});
      Matchers.Add(new Matcher(){Regex="^(4)$", Token="Four",Mod=4});
      Matchers.Add(new Matcher(){Regex="^(3)$", Token="Three",Mod=3,AltToken="Thir"});
      Matchers.Add(new Matcher(){Regex="^(2)$", Token="Two",Mod=2});
      Matchers.Add(new Matcher(){Regex="^(1)$", Token="One", Mod=1});
   }
   public List<string> Solve()
   {
       TestNumber = Number;
       var results = new List<String>();
       foreach(var matcher in Matchers)
       {
          var result = Pass(TestNumber, matcher);
          if(result.Equals("NoMatch")break;
          if(result.Equals("Anomaly"){Console.WriteLine(String.Format("There was an anomaly parsing {0}, TestNumber.ToString());break;}
          results.Add(result);
       }
       return results;
   }
   public string Pass(long number, string matcher)
   {
       string test = number.ToString();
       string count = string.Empty;
       MatchCollection matches = Regex.Matches(test, matcher);
       if(matches.Count == 0) return "NoMatch";
       if(matches.Count > 1) return "Anomaly";

       TestNumber %= matcher.Mod;
       count = Math.Floor(TestNumber).ToString();

       while(TestNumber >= matcher.Mod)
       {
         TestNumber -= matcher.Mod;
       }
       return String.Format("{0} {1}", count, matcher.Token);
   }
}

public class Matcher
{
   public string Regex{get;set;}
   public string Token{get;set;}
   public string AltToken{get;set;}
   public long Mod{get;set;
}
\$\endgroup\$
  • \$\begingroup\$ I'm not sure why a "long answer" would be required. The code is self-documenting \$\endgroup\$ – K. Alan Bates Oct 24 '14 at 14:25
  • \$\begingroup\$ Your first revision didn't specify anything about the code, but now it does, so I'll remove the notice. \$\endgroup\$ – Jamal Oct 24 '14 at 17:21
  • \$\begingroup\$ This is not homework. I'm just hacking for fun. \$\endgroup\$ – Raz Megrelidze Oct 24 '14 at 18:16
2
\$\begingroup\$

Well, first off, it's much better to wrote actual unit tests instead of manually verifying by reading output from the console. This is a bit of a tricky task, so it would be nice to know we're getting the correct output as we make changes to the method.

The second thing I notice is that you repeat this logic a lot.

builder.AppendFormat("{0} Billion ",NumberToEnglish(n / billion));
n -= (n / billion) * er.AppendFormat("{0} Million ",NumberToEnglish(n / million));

I would consider creating a private function that encapsulates it.


I would like to commend you on a very nice use of recursion though. While this could be done in a loop, the stack for this one shouldn't grow large, and the recursive way makes for cleaner code in my opinion.

\$\endgroup\$
  • \$\begingroup\$ I could create the private method, but i'm not sure how to name it. \$\endgroup\$ – Raz Megrelidze Oct 24 '14 at 11:18
  • 1
    \$\begingroup\$ Honestly @RezoMegrelidze.... I'm drawing a blank on the name too. \$\endgroup\$ – RubberDuck Oct 24 '14 at 13:08
2
\$\begingroup\$

I have done similar things like this before by using extentions.

namespace ExtensionMethods
{
    public static class MyExtensions
    {
        public static String ToHumanReadablke(this int theInt)
        {
           //Your logic here
        }
    }   
}

Take note of the namespace. You can define your own and then on each page you would just use the using directive; using ExtenstionMethods but I usually put extensions that work on base C# things like string or int into my projects namespace, that that the extensions is available, always. Intelisense in VS 2010+, is pretty intelligent and will show you waht you need.

Then anybody in the project, new, old or unexperienced will notice this simple one liner.

{
  public ActionResult Blah(int theBigNumber)
  {
     return Content(theBigNumber.ToHumanReadable());
  }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.