Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Following up on Davidmh's answer, here are some other improvements:

##Consistency:##

Consistency:

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

##Code Style:##

Code Style:

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _create_number(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

Following up on Davidmh's answer, here are some other improvements:

##Consistency:##

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

##Code Style:##

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _create_number(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

Following up on Davidmh's answer, here are some other improvements:

Consistency:

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

Code Style:

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _create_number(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

Fixed answer to make function name more pythonic
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48

Following up on Davidmh's answer, here are some other improvements:

##Consistency:##

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

##Code Style:##

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _createNumber_create_number(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

Following up on Davidmh's answer, here are some other improvements:

##Consistency:##

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

##Code Style:##

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _createNumber(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

Following up on Davidmh's answer, here are some other improvements:

##Consistency:##

Instead of using tuples for only tens and zero_to_nineteen, stay consistent and use it for the suffixes as well, by making it 1000's based. Aka, the indices would correspond to the power of a thousand that the number relates to. For example, 1000 could be 1st element, while 10 ** 30 would be the 10th.

##Code Style:##

Your code repeatedly utilizes blocks of the format:

a = number // x
b = number % x

This can be simplified down using Python's built-int divmod function like so:

a, b = divmod(number, x)

Also, your code repeatedly uses code segments like this:

if second_part == 0:
 return first_part
else:
 return first_part + separator + second_part

You are violating the DRY principle by doing this and should instead abstract this logic out into a function of its own like so:

def _create_number(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

Other Suggestions:

Normally, numbers are expected to be separated by commas. So one million three hundred thousand forty-five would be one million, three hundred thousand, forty-five. This is easy to do with the suggested function, as you simply make the seperator ", " instead of " ".

Instead of fumbling around with floats and such, let Python handle it via int() and then the only thing you would want to catch is when that throws an OverflowError for large floating point values (like 1e584). This is done like so:

try:
 number = int(number)
except OverflowError:
 # This will be triggered if trying to use this with numbers like 1e584
 return "infinity"

Python will raise a ValueError automatically anyways if it's not something that can be interpreted by int(). This code also has the benefit of being a bit more future-proof. If int() will start to work with tuples or other data types someday (aka (1, 2, 3) could become 123), then this code will still work.

Lastly, you could add a special case for a thousand, and then remove the -illion from each of the suffixes for a more compact form.

deleted 2955 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48

##Code:##

Here is my final result:

from math import log
zero_to_nineteen = ("zero", "one", "two", "three", "four",
 "five", "six", "seven", "eight", "nine",
 "ten", "eleven", "twelve", "thirteen", "fourteen",
 "fifteen", "sixteen", "seventeen", "eighteen", "nineteen")
tens = ("zero", "ten", "twenty", "thirty", "forty",
 "fifty", "sixty", "seventy", "eighty", "ninety")
# Powers of a thousand (US system of naming)
suffixes = ("zero", "thousand", "million", "billion", "trillion",
 "quadrillion", "quintillion", "sextillion", "septillion",
 "octillion", "nonillion", "decillion", "undecillion",
 "duodecillion", "tredecillion", "quattuordecillion",
 "quinquadecillion", "sedecillion", "septendecillion",
 "octodecillion", "novendecillion", "vigintillion",
 "unvigintillion", "duovigintillion", "tresvigintillion",
 "quattuorvigintillion", "quinquavigintillion", "sesvigintillion",
 "septemvigintillion", "octovigintillion", "novemvigintillion",
 "trigintillion", "untrigintillion", "duotrigintillion",
 "trestrigintilion", "quattuortrigintillion",
 "quinquatrigintillion", "sestrigintillion", "septentrigintillion",
 "octotrigintillion", "noventrigintillion", "quadragintillion")
def spell_out(number):
 """Returns a string representation of the number, in the US system"""
 try:
 number = int(number)
 except OverflowError:
 # This will be triggered with large float values such as 1e584
 return "infinity"
 if number < 0:
 return "negative " + spell_out(-1 * number)
 if number < 20:
 return zero_to_nineteen[number]
 if number < 100:
 tens_digit, ones_digit = divmod(number, 10)
 return _createNumber(tens[tens_digit], ones_digit, "-")
 if number < 1000:
 hundreds_digit, rest = divmod(number, 100)
 return _createNumber(spell_out(hundreds_digit) + " hundred", rest)
 suffix_index = int(log(number, 1000))
 if suffix_index < len(suffixes):
 suffix_value = 1000 ** suffix_index
 prefix, rest = divmod(number, suffix_value)
 prefix = spell_out(prefix) + " " + suffixes[suffix_index]
 return _createNumber(prefix, rest, ", ")
 return "infinity"
def _createNumber(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

##Code:##

Here is my final result:

from math import log
zero_to_nineteen = ("zero", "one", "two", "three", "four",
 "five", "six", "seven", "eight", "nine",
 "ten", "eleven", "twelve", "thirteen", "fourteen",
 "fifteen", "sixteen", "seventeen", "eighteen", "nineteen")
tens = ("zero", "ten", "twenty", "thirty", "forty",
 "fifty", "sixty", "seventy", "eighty", "ninety")
# Powers of a thousand (US system of naming)
suffixes = ("zero", "thousand", "million", "billion", "trillion",
 "quadrillion", "quintillion", "sextillion", "septillion",
 "octillion", "nonillion", "decillion", "undecillion",
 "duodecillion", "tredecillion", "quattuordecillion",
 "quinquadecillion", "sedecillion", "septendecillion",
 "octodecillion", "novendecillion", "vigintillion",
 "unvigintillion", "duovigintillion", "tresvigintillion",
 "quattuorvigintillion", "quinquavigintillion", "sesvigintillion",
 "septemvigintillion", "octovigintillion", "novemvigintillion",
 "trigintillion", "untrigintillion", "duotrigintillion",
 "trestrigintilion", "quattuortrigintillion",
 "quinquatrigintillion", "sestrigintillion", "septentrigintillion",
 "octotrigintillion", "noventrigintillion", "quadragintillion")
def spell_out(number):
 """Returns a string representation of the number, in the US system"""
 try:
 number = int(number)
 except OverflowError:
 # This will be triggered with large float values such as 1e584
 return "infinity"
 if number < 0:
 return "negative " + spell_out(-1 * number)
 if number < 20:
 return zero_to_nineteen[number]
 if number < 100:
 tens_digit, ones_digit = divmod(number, 10)
 return _createNumber(tens[tens_digit], ones_digit, "-")
 if number < 1000:
 hundreds_digit, rest = divmod(number, 100)
 return _createNumber(spell_out(hundreds_digit) + " hundred", rest)
 suffix_index = int(log(number, 1000))
 if suffix_index < len(suffixes):
 suffix_value = 1000 ** suffix_index
 prefix, rest = divmod(number, suffix_value)
 prefix = spell_out(prefix) + " " + suffixes[suffix_index]
 return _createNumber(prefix, rest, ", ")
 return "infinity"
def _createNumber(prefix, suffix, seperator=" "):
 # Returns a number with given prefix and suffix (if needed)
 if not isinstance(prefix, str):
 prefix = spell_out(prefix)
 return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)
Added a comment to code
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
added 137 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
lang-py

AltStyle によって変換されたページ (->オリジナル) /