Skip to content

Conversation

@ionite34
Copy link

Issue

The original statement (at line 115 of numbers.py) of:

if number > 1000 < 10000 and (number % 100 == 0) and (number % 1000 != 0):

is an invalid numerical comparison which translates to (number > 1000) and (1000 < 10000). In effect only number > 1000
is evaluated, as 1000 < 10000 is always True.

Affected Public Methods

The primary public method of the file, normalize_numbers() currently returns an incorrect conversion for numbers that are greater than 10,000 which also satify the conditions (number % 100 == 0) and (number % 1000 != 0).

normalize_numbers('15,500') >>> 'one hundred and fifty-five hundred'

Proposed fix

The proposed fix is to use the correct mathematical form of the comparison as documented in https://docs.python.org/3/reference/expressions.html#comparisons

if 1000 < number < 10000 and (number % 100 == 0) and (number % 1000 != 0):

This results in the correct conversion of numbers greater than 10000, divisible by 100, and indivisible by 1000:

normalize_numbers('15,500') >>> 'fifteen thousand five hundred'
The original comparison of `if number > 1000 < 10000` is an invalid comparison usage as it translates to '(number > 1000) and (1000 < 10000)`. In effect only `number > 1000` is evaluated as `1000 < 10000` is always True.
@ionite34 ionite34 marked this pull request as ready for review May 18, 2022 16:42
@FinAminToastCrunch
Copy link

Looks good to me. Nice catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants