4
\$\begingroup\$

Context:

Convert a non-negative integer num to its English words representation.

Example 1:

Input: num = 123 Output: "One Hundred Twenty Three"

Example 2:

Input: num = 12345 Output: "Twelve Thousand Three Hundred Forty Five"

Example 3:

Input: num = 1234567 Output: "One Million Two Hundred Thirty Four Thousand Five Hundred Sixty Seven"

Constraint: 0 < num <= (2^31 )- 1

I tried coding this exercise without external libraries and came up with the solution below. Please give me feedback on which areas in my code I can make improvements and make it better in terms of code style, logic and performance.

 integer = input(
    "Enter non-negative integer  number which should not begin with zero")
size = len(integer)

# Created three dicts for matching numbers as keys to corresponding words
dic_ones = {1: "One", 2: "Two", 3: "Three", 4: "Four",
            5: "Five", 6: "Six", 7: "Seven", 8: "Eight", 9: "Nine"}
dic_tens = {10: " Ten ", 11: "Eleven", 12: " Twelve ", 13: " Thirteen ", 14: " Fourteen ", 15: " Fifteen ", 16: " Sixteen ",
            17: " Seventeen ", 18: " Eighteen ", 19: " Nineteen "}
dic_tys = {20: " Twenty ", 30: " Thirty ", 40: " Forty ", 50: " Fifty ",
           60: " Sixty ", 70: " Seventy ", 80: " Eighty ", 90: " Ninety "}


# Created one main function digits_length within which I called functions(ones,tens,hundreds,thousands,millions,billions)
# based on the input length
def ones(num):
    print(dic_ones[num], end="")


def tens(num):
    if num//10 == 1:
        print(dic_tens[num], end="")
    elif num % 10 == 0:
        print(dic_tys[num], end="")
    else:
        print(dic_tys[num-(num % 10)], end="")
        ones(num % 10)


def hundreds(num):
    if num % 100 == 0:
        ones(num//100)
        print(" Hundred")
    else:
        ones(num//100)
        print(" Hundred", end="")
        digits_length(len(str(num % 100)), num % 100)


def billions(num):
    ones(num//1000000000)
    print(" Billion", end="")
    if num % 1000000000 != 0:
        digits_length(len(str(num % 1000000000)), num % 1000000000)


def millions(num):
    digits_length(len(str(num//1000000)), num//1000000)
    print(" Million", end="")
    if num % 1000000 != 0:
        digits_length(len(str(num % 1000000)), num % 1000000)


def thousands(num):
    digits_length(len(str(num//1000)), num//1000)
    print(" Thousand", end="")
    if num % 1000 != 0:
        digits_length(len(str(num % 1000)), num % 1000)


def digits_length(length, number):
    if length == 1:
        ones(number)
    elif length == 2:
        tens(number)
    elif length == 3:
        hundreds(number)
    elif 7 > length > 3:
        thousands(number)
    elif 10 > length > 7:
        millions(number)
    else:  # else applies to length 10 or billions
        billions(number)


# ones and tens are the smallest magnitudes so they do not include recursive call to digits_length
digits_length(size, int(integer))
\$\endgroup\$
15
  • 5
    \$\begingroup\$ I'm surprised the specification omits 0. \$\endgroup\$ Commented Aug 31, 2024 at 14:34
  • 2
    \$\begingroup\$ Next exercise: write the inverse function, that converts English like "one hundred and twenty-three" into integers ;-) (it's actually simpler) \$\endgroup\$
    – Stef
    Commented Sep 1, 2024 at 21:48
  • 1
    \$\begingroup\$ Many mathematicians will leave out the word "and" from numbers like 123. It makes it possible to distinguish "123/200" from "100 23/200". \$\endgroup\$ Commented Sep 1, 2024 at 22:07
  • 1
    \$\begingroup\$ @TobySpeight re: "and"... The debate rages. imo, a beginner task should be as straightforward as possible... You may as well ask if the 'house number' of the Whitehouse is "sixteen hundred" or "one thousand six hundred"... Remember KISS principles... \$\endgroup\$
    – Fe2O3
    Commented Sep 2, 2024 at 9:25
  • 1
    \$\begingroup\$ @AdrianMcCarthy: It's a bug in the specification, the description "Convert a non-negative integer" conflicts with the stated constraint "num is strictly greater than zero" \$\endgroup\$
    – Ben Voigt
    Commented Sep 2, 2024 at 21:46

3 Answers 3

8
\$\begingroup\$

In addition to the good suggestions in the previous answer, here are some more things to consider.

Bug

When I run your code and give the "Example 3" input (1234567), the code exits with errors and does not produce the output shown in the question. I tried with a couple versions of python 3.x, and the error persists. I think the fix is to change:

elif 10 > length > 7:

to:

elif 10 > length > 6:

But, you should test it out more thoroughly.

Also, when I give the "Example 2" input (12345), the words match your question output, but the spacing is incorrect:

Twelve  ThousandThree Hundred Forty Five

There is extra space before and after Twelve and no space after Thousand.

Input prompt

It is customary to add a space at the end of the prompt text to distinguish it from the user input number. This is what it looks like:

Enter non-negative integer  number which should not begin with zero12345

There is also extra space after integer. Here is modified code:

integer = input(
    "Enter non-negative integer number which should not begin with zero ")

What it looks like:

Enter non-negative integer number which should not begin with zero 12345

Output

When I run the code, the output is missing a newline character, which causes the output to be merged with the command prompt in my Linux shell. The digits_length function should probably make sure there is a trailing newline. I just did this as a hack:

digits_length(size, int(integer))
print()

Input checking

It would be nice to add checks of the user input to more gracefully exit the code. For example, it dies horribly when entering an 11-digit number (exceeding your constraint) or when entering non-numeric input like 44r (mistakenly hitting a letter key).

Layout

The code is laid out very well for the most part. However, the 3 dicts at the top are difficult to read, especially with long lines. You could use the black program to automatically apply formatting to the code.

You should break the long comment line into multiple lines to make it easier to read:

# Created one main function digits_length within which I called 
# functions (ones,tens,hundreds,thousands,millions,billions)
# based on the input length

Readability

For large numbers like 1000000, it is nice to use the underscore separator: 1_000_000.

Documentation

Add a docstring to the top of the code to describe its purpose

"""
Convert a non-negative integer num to its English words representation.

Example:

Input: num = 123 Output: "One Hundred Twenty Three"
"""
\$\endgroup\$
0
10
\$\begingroup\$

First, the spec itself is not very good. Conventionally, English numerals are never written out in Title Case so it doesn't make sense to code for that. Also, the spec is missing the hyphen and negative cases (though we can forgive this because those would introduce more complexity than the problem intends).

You don't need to use dictionaries. In all cases, you can write out sequences of strings and infer their meaning by index. Those sequences should be tuples for immutability.

Having separate functions for millions, billions etc. is an example of inappropriate loop unrolling. A simpler algorithm would loop through order names.

Notice that you're doing a lot of num // 100; num % 100; these expressions should be replaced with a call to divmod.

Don't force a print at every step. Instead, return a string. If you form this using an iterator, then it makes a lot of the code simpler.

Add unit tests. Those would have caught the bug the other answers have correctly described.

All together,

import math
import typing


# https://en.wikipedia.org/wiki/English_numerals

ONES = (
    # cardinals
    'zero', 'one', 'two', 'three', 'four',
    'five', 'six', 'seven', 'eight', 'nine',
    # not cardinals, but treated the same
    'ten', 'eleven', 'twelve', 'thirteen', 'fourteen',
    'fifteen', 'sixteen', 'seventeen', 'eighteen', 'nineteen',
)

TENS = (
    'zero', 'ten',  # not used; included for index consistency
    'twenty', 'thirty', 'forty',
    'fifty', 'sixty', 'seventy', 'eighty', 'ninety',
)

HUNDRED = 'hundred'  # to keep all of the strings together here

THOUSANDS = (
    'one', 'thousand', 'million', 'billion',
    # these are not necessary by the spec, but easy to define
    'trillion', 'quadrillion', 'quintillion', 'sextillion', 'septillion', 'octillion', 'nonillion',
    'decillion',
    # etc.
)


def format_fragments(x: int) -> typing.Iterator[str]:
    if x <= 0:
        yield ONES[0]
        return

    while x > 0:
        exp_thousand = int(math.log10(x))//3
        rem100, x = divmod(x, 1000**exp_thousand)

        x100, rem10 = divmod(rem100, 100)
        if x100 > 0:
            yield from (ONES[x100], HUNDRED)

        if rem10 > 0:
            x10, rem1 = divmod(rem10, 10)
            if x10 >= 2:
                yield TENS[x10]
                if rem1 > 0:  # actual cardinals
                    yield ONES[rem1]
            else:  # teens case
                yield ONES[rem10]

        if exp_thousand > 0:
            yield THOUSANDS[exp_thousand]


def format_english_number(x: int) -> str:
    """
    Format a non-negative integer in English numerals. The output is lowercase and does not include
    'and' or hyphens. Negative integers are coerced to 'zero'.
    """
    return ' '.join(format_fragments(x))


def test_stock() -> None:
    assert format_english_number(123) == 'one hundred twenty three'
    assert format_english_number(12_345) == 'twelve thousand three hundred forty five'
    assert format_english_number(1_234_567) == 'one million two hundred thirty four thousand five hundred sixty seven'


def test_single_order_present() -> None:
    fmt = format_english_number
    assert fmt(-10) == 'zero'
    assert fmt(0) == 'zero'
    assert fmt(1) == 'one'
    assert fmt(18) == 'eighteen'
    assert fmt(80) == 'eighty'
    assert fmt(800) == 'eight hundred'
    assert fmt(8_000) == 'eight thousand'
    assert fmt(18_000) == 'eighteen thousand'
    assert fmt(8_000_000) == 'eight million'
    assert fmt(80_000_000) == 'eighty million'


def test_two_orders_present() -> None:
    fmt = format_english_number
    assert fmt(74) == 'seventy four'
    assert fmt(107) == 'one hundred seven'
    assert fmt(740) == 'seven hundred forty'
    assert fmt(7_004) == 'seven thousand four'
    assert fmt(7_040) == 'seven thousand forty'
    assert fmt(7_400) == 'seven thousand four hundred'
    assert fmt(170_000) == 'one hundred seventy thousand'


def test_long() -> None:
    actual = format_english_number(303_820_063_444_912)
    expected = (
        'three hundred three trillion'
        ' eight hundred twenty billion'
        ' sixty three million'
        ' four hundred forty four thousand'
        ' nine hundred twelve'
    )
    assert actual == expected


if __name__ == '__main__':
    test_stock()
    test_single_order_present()
    test_two_orders_present()
    test_long()
\$\endgroup\$
3
  • \$\begingroup\$ "English numerals are never written out in Title Case"? They are when found in titles. "Twenty Thousand Leagues Under the Seas" So it may make sense to have an option argument as_title_case rather than assert it is wrong. \$\endgroup\$
    – Ben Voigt
    Commented Sep 2, 2024 at 21:38
  • 1
    \$\begingroup\$ @BenVoigt Perhaps more accurately, it's a bad default. When str.title() exists, it makes no sense to include that as a deliberate feature. \$\endgroup\$
    – Reinderien
    Commented Sep 2, 2024 at 23:16
  • 2
    \$\begingroup\$ 100% agreed it's a poor default \$\endgroup\$
    – Ben Voigt
    Commented Sep 3, 2024 at 14:37
5
\$\begingroup\$

My two cents:

class FormatNumber():
    @staticmethod
    def create_maps(start=0, stop=10, skip=None, names=()):
        if skip:
            return {k: v for k, v in zip(range(start, stop, skip), names)}
        return {k: v for k, v in zip(range(start, stop), names)}

    # Created three dicts for matching numbers as keys to corresponding words
    dict_ones = create_maps(1, 10, names=["One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine"])

    dict_tens = create_maps(10, 20,
                            names=[" Ten ", " Eleven ", " Twelve ", " Thirteen ", " Fourteen ", " Fifteen ", " Sixteen ",
                                   " Seventeen ", " Eighteen ", " Nineteen "])
    dict_tys = create_maps(20, 100, 10,
                           names=[" Twenty ", " Thirty ", " Forty ", " Fifty ", " Sixty ", " Seventy ", " Eighty ",
                                  " Ninety "])

    def __init__(self, number):
        self.num = number

    def ones(self, num):
        print(self.dict_ones[num], end="")

    def tens(self, num):
        if num // 10 == 1:
            print(self.dict_tens[num], end="")
        elif num % 10 == 0:
            print(self.dict_tys[num], end="")
        else:
            print(self.dict_tys[num - (num % 10)], end="")
            self.ones(num % 10)

    def hundreds(self, num):
        # print(type(num))
        if num % 100 == 0:
            self.ones(num // 100)
            print(" Hundred")
        else:
            self.ones(num // 100)
            print(" Hundred", end="")
            self._format_number(num % 100)

    def billions(self, num):
        self.ones(num // 1000000000)
        print(" Billion", end="")
        if num % 1000000000 != 0:
            self._format_number(num % 1000000000)

    def millions(self, num):
        self._format_number(num // 1000000)
        print(" Million", end="")
        if num % 1000000 != 0:
            self._format_number(num % 1000000)

    def thousands(self, num):
        self._format_number(num // 1000)
        print(" Thousand", end="")
        if num % 1000 != 0:
            self._format_number(num % 1000)

    def _format_number(self, number):
        length = len(str(number))
        if length == 1:
            self.ones(number)
        elif length == 2:
            self.tens(number)
        elif length == 3:
            self.hundreds(number)
        elif 7 > length > 3:
            self.thousands(number)
        elif 10 > length > 7:
            self.millions(number)
        else:  # else applies to length 10 or billions
            self.billions(number)

    def format_number(self):
        self._format_number(self.num)


if __name__ == '__main__':
    integer = int(input("Enter non-negative integer  number which should not begin with zero"))
    english_repr = FormatNumber(integer)
    english_repr.format_number()

Here, I improved a few things:

  • Refactored all the number printing functions (i.e., ones, tens, hundreds, ...) into a class.
  • Added a __init__() method that takes one argument, which is the number to be converted into English. Store this number inside attribute self.num for ease of use inside methods.
  • Added a utility function called create_maps that helps with the number-word correlation hash map generation. (Technically, this is a staticmethod, created using the @staticmethod decorator; static methods act like functions, since they do not receive the implicit first argument self). It takes four optional arguments start, stop, skip, and names. skip is used for the -tys, skipping 10 each time. The other three arguments are rather trivial. Using dictionary comprehension combined with range() and zip() to make work easier and code shorter.
  • Replaced the originally hard-coded dic_ones, dic_tens and dic_tys with newly created dictionaries generated by the create_maps function. Also renamed these variables to dict_ones, dict_tens, and dict_tys (adding "t" in the middle to enhance readability), respectively.
  • Since the maps are now class variables, we use self. to get them in our methods.
  • Created two methods to replace the original digits_length function. Renamed them _format_number and format_number, the former an internal implementation replacing the digits_length function, and the latter to be used directly by outside code. Note the use of a single underscore _ in the internal implementation method's name, but otherwise identical with the interface (a method used directly by outside code; in this case it's format_number) method's name. This is common and good software engineering practice, especially in Python, where there's no such thing as a public or private method.
  • In the _format_number method, removed parameter length. There's no need for it, since we can just use the line length = len(str(number)) to find the length. This saves a lot of repeating and redundant code in the number formatting methods when calling _format_number.
  • The format_number method simply calls the _format_number method with self.num passed-in.
  • At the bottom of the file, use the famous Python line if __name__ == '__main__': to differentiate between being run as a script or being imported as a library. This enables the FormatNumber class to be exported and used by other files, while also conducting a simple unit testing of the functionality when run as a script.
  • Finally, use input() to prompt the user for a number, just like in the original code. Instantiates the FormatNumber class and initializes the new object with the user input converted to an integer. At last, call the object's format_number method to run the main logic. Omit the size = len(integer) line, since again the length can be computed internally and thus does not need to be passed-in.

That wraps-up the code! Note that my code did not decrease the number of lines, and instead actually increases the line count by around 10 lines. However, this code greatly improves upon the readability, reusability, maintainability, and ease of debugging of the code, and also enables seamless integration with other files in a potentially larger project, which are all extremely crucial aspects of software engineering. Sometimes decreasing the line count isn't the topmost priority, and everyone should opt for those virtues listed above instead.

Hope this helps!

\$\endgroup\$
7
  • 4
    \$\begingroup\$ Your class is confusing. It has a field named num but also has a num parameter passed into each method. Every method behaves as a static method. \$\endgroup\$
    – Nayuki
    Commented Oct 1, 2024 at 1:32
  • \$\begingroup\$ @Nayuki You're mistaken. The methods are not static. They are normal methods. Also it's common practice to name numbers num, and although it is indeed a bit sloppy in letting the attribute and the parameters all have the same name, any reasonably experienced programmer (and I assume OP is) would be able to read the code without any problems. It is not confusing. ... \$\endgroup\$
    – Luke L
    Commented Oct 1, 2024 at 4:04
  • 6
    \$\begingroup\$ It's hard to explain the deficiencies of your code in a comment. Put your code up for review if you want an explanation. \$\endgroup\$
    – Nayuki
    Commented Oct 1, 2024 at 23:24
  • 3
    \$\begingroup\$ The code from this answer was used for a question on softwareengineering.stackexchange. \$\endgroup\$
    – Doc Brown
    Commented Oct 2, 2024 at 5:32
  • 3
    \$\begingroup\$ Posted on Meta. \$\endgroup\$
    – Peilonrayz
    Commented Oct 2, 2024 at 16:40

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.