7
\$\begingroup\$

The question itself should be fairly self-explanatory. My method takes in value, max_digits, and decimal_places, and returns a formatted decimal number string. The code:

from decimal import Decimal, ROUND_DOWN

def format_to_decimal_string(value, max_digits, decimal_places):
    """Formats a decimal number to the given number of decimal places.

    Notes:
        - If the decimal number is equal to zero, the absolute of the decimal 
        number is formatted and returned to remove the minus sign. See the 
        first doctest for an example.
        - If the number of digits in the rounded up decimal number exceeds the 
        `max_digits`, the method tries to return the rounded down version. See 
        the second doctest for an example.

    Doctests:
        >>> format_to_decimal_string('-0.000', 4, 2)
        '0.00'
        >>> format_to_decimal_string('99.999999', 4, 2)
        '99.99'
        >>> format_to_decimal_string('99.999999', 5, 2)
        '100.00'
        >>> format_to_decimal_string('420.2108', 5, 0)
        '420'
        >>> format_to_decimal_string('foo', 4, 2)
        Traceback (most recent call last):
            ...
        TypeError: The value 'foo' is not a valid decimal number.
        >>> format_to_decimal_string('120.411', 4, 2)
        Traceback (most recent call last):
            ...
        ValueError: After formatting, the number of digits in the value 
        '120.41' is 5, which exceeds the maximum number of digits of 4.
    """
    try:
        decimal_number = Decimal(str(value))
    except:
        raise TypeError(
            'The value \'{}\' is not a valid decimal number.'.format(value)
        )

    if decimal_number == 0:
        return str(abs(decimal_number).quantize(
            Decimal(10) ** -decimal_places)
        )

    rounded_up = decimal_number.quantize(Decimal(10) ** -decimal_places)
    if not len(rounded_up.as_tuple().digits) > max_digits:
        return str(rounded_up)

    rounded_down = decimal_number.quantize(
        Decimal(10) ** -decimal_places,
        rounding=ROUND_DOWN
    )
    rounded_down_digits_length = len(rounded_down.as_tuple().digits)
    if not rounded_down_digits_length > max_digits:
        return str(rounded_down)
    else:
        raise ValueError(
            'After formatting, the number of digits in the value \'{}\' is ' \
            '{}, which exceeds the maximum number of digits of {}.'.format(
                rounded_down, rounded_down_digits_length, max_digits
            )
        )

Please assume that max_digits, and decimal_places will always be positive integers, and max_digits is greater than decimal_places, because these are coming from my database, so no validation is required for these arguments.

The comments should also be self explanatory. We only want to round up if the rounding up does not exceed the given maximum number of digits. Would really appreciate a review of this method.

\$\endgroup\$
4
\$\begingroup\$

Your code looks quite good to me. Good structure, consistent naming, appropriate documentation.

There are just some small ideas I would like to offer to you. Since your question is specifically tagged for Python 3, you might make use of the new f-string synthax instead of format. So your error messages would become something like

raise TypeError(
    f"The value '{value}' is not a valid decimal number."
)

which I think looks even nicer. I also replaced the outer ' with " so one does not have to escape the inner ' with backslashes.

The last error message could use the same tricks and could also utilize implicit line continuation in function parenthesis instead of manual line continuation triggered by \. This would ultimately lead to the following snippet:

raise ValueError(
    f"After formatting, the number of digits in the value '{rounded_down}' "
    f"is {rounded_down_digits_length}, which exceeds the maximum number of "
    f"digits of {max_digits}."
)

I was thinking about to address the idea of adding doctests to your code, but you have added this feature in an edit that took place while is was writing this up. So consider this to be done.

\$\endgroup\$
1
\$\begingroup\$

Some other minor issues:

Invert your logic

if not len(rounded_up.as_tuple().digits) > max_digits:

should be

if len(rounded_up.as_tuple().digits) <= max_digits:

Lose the redundant else

This:

if not rounded_down_digits_length > max_digits:
    return str(rounded_down)
else:
    raise ...

can just be

if rounded_down_digits_length <= max_digits:
    return str(rounded_down)
raise ...
\$\endgroup\$
  • \$\begingroup\$ Can you tell my why the inversion of logic is necessary? Is this mentioned somewhere in some best practices doc or something? \$\endgroup\$ – darkhorse Mar 31 at 14:27
  • \$\begingroup\$ It's not necessary, but it is more legible. In the modified syntax, you don't need to write a not. I can't really find a reference for this, but I stand by it. \$\endgroup\$ – Reinderien Mar 31 at 15:34

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.