Number to Words

Question

I have to convert positive numbers, that are less than 1000, to words. A quick search didn't find anything that looked good to my eye, so I came up with the following. While I was at it I expanded the conversion. I am looking for comments, suggestions, or errors I am missing.

Public Class NumberToWords

    Public Shared Function Convert(num As Integer, Optional negativeprfx As String = "negative") As String

        Dim wnum As Integer = num 'working number

        If wnum = Integer.MinValue Then
            Throw New ArgumentException("Can't convert")
        End If

        If wnum < 0 Then wnum = -wnum 'convert to positive if needed

        Dim rv As New System.Text.StringBuilder

        'is the number known?
        rv.Append(Defined(wnum))
        If rv.Length > 0 Then Return rv.ToString 'yes, return it

        'the groups
        Dim units As Integer = 0
        Dim tens As Integer = 0
        Dim hunds As Integer = 0
        Dim thous As Integer = 0
        Dim mils As Integer = 0
        Dim bils As Integer = 0

        'get count of each grouping
        'decreasing wnum by the grouping total

        bils = wnum \ NumWords.billion
        wnum -= bils * NumWords.billion

        mils = wnum \ NumWords.million
        wnum -= mils * NumWords.million

        thous = wnum \ NumWords.thousand
        wnum -= thous * NumWords.thousand

        hunds = wnum \ NumWords.hundred
        wnum -= hunds * NumWords.hundred

        'special case for tens
        'if the number is less than 21
        'don't bother dividing by ten
        'because all numbers less than 21 are defined

        If wnum > 20 Then
            tens = wnum \ NumWords.ten
            wnum -= tens * NumWords.ten
        End If

        units = wnum

        'now check each group
        'and recursively call convert on the
        'groups amount

        If bils > 0 Then
            rv.Append(Convert(bils))
            rv.Append(" ")
            rv.Append(NumWords.billion.ToString)
            rv.Append(" ")
        End If

        If mils > 0 Then
            rv.Append(Convert(mils))
            rv.Append(" ")
            rv.Append(NumWords.million.ToString)
            rv.Append(" ")
        End If

        If thous > 0 Then
            rv.Append(Convert(thous))
            rv.Append(" ")
            rv.Append(NumWords.thousand.ToString)
            rv.Append(" ")
        End If

        If hunds > 0 Then
            rv.Append(Convert(hunds))
            rv.Append(" ")
            rv.Append(NumWords.hundred.ToString)
            rv.Append(" ")
        End If

        If tens > 0 Then
            'the tens (10, 20, 30 ...) are defined
            rv.Append(Convert(tens * NumWords.ten)) ' times ten to get the defined number string
            rv.Append(" ")
        End If

        If units > 0 Then
            rv.Append(Convert(units))
            rv.Append(" ")
        End If
        'if the number was negative fix return
        If num < 0 Then
            rv.Insert(0, " ")
            rv.Insert(0, negativeprfx)
        End If
        Return rv.ToString.Trim
    End Function

    Private Shared Function Defined(num As Integer) As String
        'is the number defined and less than 100
        If [Enum].IsDefined(GetType(NumWords), num) AndAlso num < 100 Then
            'yes
            Return [Enum].GetName(GetType(NumWords), num)
        Else
            'no
            Return ""
        End If
    End Function

    Enum NumWords
        zero = 0
        one = 1
        two = 2
        three = 3
        four = 4
        five = 5
        six = 6
        seven = 7
        eight = 8
        nine = 9
        ten = 10
        eleven = 11
        twelve = 12
        thirteen = 13
        fourteen = 14
        fifteen = 15
        sixteen = 16
        seventeen = 17
        eightteen = 18
        nineteen = 19
        twenty = 20
        thirty = 30
        forty = 40
        fifty = 50
        sixty = 60
        seventy = 70
        eighty = 80
        ninety = 90
        hundred = 100
        thousand = 1000
        million = 1000000
        billion = 1000000000
    End Enum
End Class

Answer 1

The function name Convert doesn't seem particularly helpful to me. Convert how? From what to what? In fact, we're not converting anything... we're representing the same unconverted value in a different format.

---

Dim wnum As Integer = num 'working number

If wnum is the working number, and you feel wnum is so unclear that you need to add a comment to explain what it is, then why don't you just name the variable workingNumber so it's more clear through the entirety of the function?

---

If wnum < 0 Then wnum = -wnum 'convert to positive if needed

Why not just take the absolute value?

wnum = Math.Abs(wnum)

---

Dim units As Integer = 0
Dim tens As Integer = 0
Dim hunds As Integer = 0
Dim thous As Integer = 0
Dim mils As Integer = 0
Dim bils As Integer = 0

I'd call these as: ones, tens, hundreds, thousands, millions, billions

Shorter names don't make your program run faster or use less memory. It just makes it harder to read.

---

    If num < 0 Then
        rv.Insert(0, " ")
        rv.Insert(0, negativeprfx)
    End If

I'd also recommend doing this first, for the same reason we're starting with billions and working down to the ones place. I'm not 100% familiar with how .NET's string builder workers, but generally speaking, adding things to the front of a collection (in this case, a string is a collection of characters) is significantly more expensive and time consuming then adding to the back of a collection. So let's start at the front.

Answer 2

Normally I would say that magic numbers are a bad thing and are to be avoided, but since we're explicitly talking about numbers, I think you're justified in their use here.

Why are you doing this?

    If wnum = Integer.MinValue Then
        Throw New ArgumentException("Can't convert")
    End If

The minimum value of an integer is -2,147,483,648. I don't readily see a reason why you couldn't convert that value. If anything, you would want to check for values less than Integer.MinValue or greater than Integer.MaxValue, but since the num parameter is an integer, the compiler will do that check for you.

@Nhgrif pointed out that this is an "off by one issue" that causes an exception when getting the absolute value of Integer.MinValue. I take back what I said and recommend adding a comment in the code to explain the reason for this check instead.

Also consider giving a more meaningful message than "Can not convert". "Can not convert numbers less than '-2,147,483,647'." Would be much more meaningful to someone debugging a program that calls this code.