4
\$\begingroup\$

I'd like to get a code review and some insights of whether I chose a good way to do this. What would other, clever ways look like.

"""
Convert numeric string to number withoutusing python's built in functions.
"""
dec_places = {6:100000, 5:10000, 4:1000, 3:100, 2:10, 1:1}
char_digit = {'0':0, '1':1, '2':2, '3':3, '4':4, '5':5,  '6':6, '7':7, '8':8,  '9':9}

def str2int(num_str):
    iter = len(num_str)
    number = 0
    for char in num_str:
        number += (char_digit[char] * dec_places[iter])
        iter -= 1
    return number

print(str2int('623297'))
\$\endgroup\$
  • 1
    \$\begingroup\$ Good. Clever. Choose one. \$\endgroup\$ – Jerry Coffin Dec 28 '16 at 2:50
  • 1
    \$\begingroup\$ Your return statement is outside of your function, it can't run as-is; I think the code you posted is not formatted correctly. \$\endgroup\$ – Phrancis Dec 28 '16 at 3:18
4
\$\begingroup\$

Doctstrings

The docstring:

"""
Convert numeric string to number withoutusing python's built in functions.
"""

Should be at the top of the inside of the function.

(Also withoutusing should be without using.)

Global Variables

Global variables are typically a bad design practice. Move these:

dec_places = {6:100000, 5:10000, 4:1000, 3:100, 2:10, 1:1}
char_digit = {'0':0, '1':1, '2':2, '3':3, '4':4, '5':5,  '6':6, '7':7, '8':8,  '9':9}

Inside the function.

Function naming

str2int should be renamed to str_to_int.

Better algorithm

dec_places is unneeded and limits your algorithm significantly.

I would enumerate over the reversed of your string:

for ind, char in enumerate(reversed(num_str)):
    number += char_digit[char] * 10**(ind + 1)

enumerate takes an iterable and creates tuples of (0, el1), (1, el2) ....

We reverse the string and then multiply each character by the appropriate power of 10. This allows you to go beyond the 6th power.

Alternatively, you can keep an iter value that counts up from 1. I'll leave this as an exercise. It might be faster than using reversed. You should not need a dictionary though.

\$\endgroup\$
  • \$\begingroup\$ You can even use sum to improve the for loop for speed. \$\endgroup\$ – 409_Conflict Dec 28 '16 at 8:05
3
\$\begingroup\$

You should avoid using iter as a variable name, since it happens to be the name of a built-in function. It's confusing to other Python programmers who have to read your code, and could produce a failure if you ever want to call the standard iter() function.

In any case, you don't want to have a clumsy loop that is based on char and also maintains iter. You certainly don't want an algorithm that puts an upper bound on the numbers you can handle. The standard algorithm goes like this:

CHAR_DIGIT = {'0':0, '1':1, '2':2, '3':3, '4':4, '5':5,  '6':6, '7':7, '8':8, '9':9}

def str2int(num_str):
    """
    Convert numeric string to number without using Python's built in functions.
    """
    number = 0
    for char in num_str:
        number = 10 * number + CHAR_DIGIT[char]
    return number
\$\endgroup\$
2
\$\begingroup\$

The first and most obvious problem with your function is that it cannot handle any numbers greater than 999999. As you may know, Python can handle integers much larger than that:

import sys
print(sys.maxsize)
# 9223372036854775807

This is an easy fix of course, as you can just add values to dec_places dictionary to cover the remainder of the 19 digits total. Or come up with a better algorithm, as Dair suggested.

Your function also cannot handle negative numbers, which the built-in int() function can handle. This is also a pretty easy fix. I added code comments to explain.

def str2int(num_str):
    number = 0
    negative = False
    # check for a starting minus sign
    if num_str.startswith('-'):
        negative = True
        # trim the left-most minus sign
        num_str = num_str[1:]
    # now get the iter with the len without the minus sign
    iter = len(num_str)
    for char in num_str:
        number += (char_digit[char] * dec_places[iter])
        iter -= 1
    # make the number negative
    if negative:
        number *= -1
    return number

print(str2int('623297'))
print(str2int('-623297'))
\$\endgroup\$

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.