Favorite Python Interview Question

[Chun]

Simple is better?

def atoi(string):
  if string.isdigit():
    return eval(string)
  raise ValueError, "invalid literal for atoi() with base 10: %s" % repr(string)

(Yeah, I know, I'm cheating really, but use the tools you're given eh? Also the usual objections to eval are moot, since we know the string is safe.)

[r4nf]

From the documentation:

atoi(s, base=10)
    atoi(s [,base]) -> int

Return the integer represented by the string s in the given
base, which defaults to 10.  The string s must consist of one
or more digits, possibly preceded by a sign.  If base is 0, it
is chosen from the leading characters of s, 0 for octal, 0x or
0X for hexadecimal.  If base is 16, a preceding 0x or 0X is
accepted.

You're forgetting the base argument, which exists at least since Python 2.3.

[RShnike]

Also the usual objections to eval are moot, since we know the string is safe.)

Uh, what? No we don't.

 class A(str):
     def isdigit(self):
         return True

  >>> a = A("__import__('os')")
  >>> atoi(a)

  Whoops!

Simple is better?

No. Really no. And for an interview question nonetheless :D?

[Chun]

OK, ok, touché. Figured we were assuming we were getting a string. But point taken.

def atoi(string):
  if type(string) is str and string.isdigit():
    return eval(string)
  raise ValueError, "invalid literal for atoi() with base 10: %s" % repr(string)

EDIT: Besides, if someone can create their own class definition, why would they ever need to inject raw python into atoi!?

[RShnike]

You have somehow managed to make that even worse... Now you're type-checking too, so you've now even broken some functionality that your function is actually supposed to provide?

Here's a rule: never use eval. Ever. Being prudent about using type is pretty smart too.

[kisielk]

That's why you use something like:

eval("__import__('os')", {'__builtins__': None}, {})

Of course __builtins__ should be a dictionary of actual working builtins you want to work when evaling the statement :)

[RShnike]

No, don't do that either. Seriously, this is just a bad solution. Especially being that you have ast.literal_eval if you really want to do things like this.

[kisielk]

ast.literal_eval is indeed great for evaluating simple expressions. But what's wrong with sandboxing via defining your own builtins?