6
\$\begingroup\$

This converts from character representation of integers to their decimal value:

def chrtodec(str):
    dec = 0
    base = ord('0')
    for chr in str:
        dec = dec * 10 + ord(chr) - base
    return dec

For example: "123" -> 123

\$\endgroup\$

4 Answers 4

6
\$\begingroup\$

You shouldn't use chr or str as names because they shadow the builtin chr and str methods. If you then wanted to use str() you'd be out of luck because str is now a string instead of a function. Given that your function deals with both strings and character ordinals it is not unlikely that these could be used. string is a potential improvement, it can occasionally cause trouble if you're trying to use the string module of the same name.

You should also add comments and a docstring. Docstrings are basically comments that are programmatically accessible so that other users can understand how to use your function.

def chrtodec(str):
    """Converts a string to a float using character ordinal positions."""

I think you have another big problem though, which is invalid input handling. Take a look at these:

>>> chrtodec("hello")
619663
>>> chrtodec("12.21")
11821
>>> chrtodec("523.32f")
5228374

Clearly there are problems here when you have characters other than numbers, so I think what you should do is raise a ValueError when an invalid string is passed. You already have the tools to do this figured out of course if you just check that a character's ordinal fits in the right range.

if not (ord('0') <= ord(chr) <= ord('9')):
    raise ValueError("Invalid character {} in string {},"
                     "only digits are parseable.".format(chr, str))

You could also just use chr.isdigit() as @Barry pointed out.

Also you could add support for negative numbers by checking at the start if the first character is ord('-').

negative = ord(str[0]) == ord('-')

This evaluates the expression str[0] == ord('-') and sets negative as the boolean result. Note that to make this compatible with the error handling I suggested, you should then remove the first character from str. And probably update the error message and docstring too.

if negative:
    str = str[1:]

Then just return with a ternary that checks negative.

return dec if not negative else -dec
\$\endgroup\$
0
3
\$\begingroup\$

Your code lacks modularization, This problem can be decomposed in:

  • Find the numerical value of a char.
  • Transform the chars into digits
  • Multiply all this numerical values by 10 ** position and sum them.

def char_to_integer(char):
    return ord(char) - ord('0')

The second function is an excellent place for a generator expression:

def string_to_digits(string):
    return (char_to_integer(i) for i in string)

Finally:

def digits_to_integer(digits):
    return sum(digit * 10 ** ( len(digits) - position )
        for position, digit in enumerate(digits))
\$\endgroup\$
3
  • \$\begingroup\$ @Barry thanks for commenting! This is indeed slower than OP's solution but I would avoid premature opimization in this case. I find it so much more readable. This is a single expression. Look how similar this is to the mathematical definition of a positional number system. Also this uses many built-in functions enumerate, reversed and sum thus delegating the work to Python itself as it is idiomatic \$\endgroup\$
    – Caridorc
    Sep 10, 2015 at 10:23
  • \$\begingroup\$ @Barry I refactored even more, the code has also become more testable and this is much faster to write than the original solution in my opinion \$\endgroup\$
    – Caridorc
    Sep 10, 2015 at 11:30
  • \$\begingroup\$ @Caridorc, I agree with Barry. I don't think the code is readable in first place. \$\endgroup\$
    – CodeYogi
    Sep 11, 2015 at 4:12
2
\$\begingroup\$

You don't support negative numbers. But that's actually pretty easy to add, all you need to do is:

chrtodec = int

That also has the handy ability to support arbitrary base conversions too!

\$\endgroup\$
3
  • \$\begingroup\$ Seriously! I was trying to do in the native way. I think you are kidding, right? \$\endgroup\$
    – CodeYogi
    Sep 10, 2015 at 5:03
  • \$\begingroup\$ Actually I am learning problem solving and according to me that's not a better approach. \$\endgroup\$
    – CodeYogi
    Sep 10, 2015 at 11:27
  • \$\begingroup\$ Np, I keep that in mind :) \$\endgroup\$
    – CodeYogi
    Sep 11, 2015 at 4:11
2
\$\begingroup\$

Please improve your naming!

I've come to love the "straight-to-the-point" Python syntax a lot, and I embrace the delicious syntactic sugar of the language; but I keep seeing really bad naming of things among Python programmers. Maybe try this?

def number_as_string_to_decimal(number_as_string):
    decimal = 0
    base = ord('0')
    for character in number_as_string:
        decimal = decimal * 10 + ord(character) - base
    return decimal
\$\endgroup\$
3
  • \$\begingroup\$ I agree, if you're trying to be short I'd at least use char. \$\endgroup\$ Sep 10, 2015 at 8:34
  • 2
    \$\begingroup\$ I don't think these names are better, either. The function does not convert a character to a decimal, but a string representing an integer in decimal notation to an integer. \$\endgroup\$
    – mkrieger1
    Sep 10, 2015 at 9:34
  • \$\begingroup\$ Thanks for pointing out that distinction, I changed my naming suggestions accordingly. \$\endgroup\$
    – Phrancis
    Sep 10, 2015 at 18:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.