-5

Hello there developers,

i am writing code that takes the user input and initializes a class depending on the input like in the example code below:

class X:
    def __init__(self):
       return

    def run(self):
       print("i am X")

def func1(cls):
    exec("global " + cls.lower())
    exec(cls.lower() + " = " + cls + "()")

def func2(mode_to_set):
    exec(mode_to_set.lower() + ".run()")

but as I run the code like this:

func1('X')
func2('X')

i keep getting this error:

Traceback (most recent call last):
  File "/Users/noahchalifour/Desktop/test.py", line 16, in <module>
    func2('X')
  File "/Users/noahchalifour/Desktop/test.py", line 13, in func2
    exec(mode_to_set.lower() + ".run()")
  File "<string>", line 1, in <module>
NameError: name 'x' is not defined

Can anyone help me?

N. Chalifour
  • 103
  • 10
  • 2
    The variable needs to already exist in the global scope. Also, THIS IS A TERRIBLE IDEA. Plus, you're trying to create an instance of `x`, but your class is called `X`. – Morgan Thrapp Aug 23 '16 at 20:12
  • why is it so bad? @MorganThrapp – N. Chalifour Aug 23 '16 at 20:13
  • 1
    Possible duplicate of [How do I create a variable number of variables in Python?](http://stackoverflow.com/questions/1373164/how-do-i-create-a-variable-number-of-variables-in-python) – Morgan Thrapp Aug 23 '16 at 20:14
  • 1
    It's horrible to understand, and test. And it's a huge security hole. – Daniel Aug 23 '16 at 20:18

3 Answers3

1

A much better way to instantiate a class based on user input would be to use a "factory pattern":

http://python-3-patterns-idioms-test.readthedocs.io/en/latest/Factory.html

Basically you create a class whose whole purpose is to create other classes based on a value. Some people might find that overkill, so you could also use a function that creates classes based on input.

Whatever you do though, the way you have it now, running raw, user-input strings using exec, is a bad idea. The best case scenario is that it introduces new bugs that are near-impossible to trace since they aren't actually recorded anywhere. Worst case scenario, a user somehow finds a way to send a string to the function, you've pretty much destroyed whatever security you've hoped for.

Basically "exec" should generally be a last resort. There are usually more elegant and secure ways to solve the problem.

TripleD
  • 199
  • 1
  • 15
0

It seems like you'd be better off having func2 instantiate and run the method:

def func2(mode_to_set):
    globals()[mode_to_set]().run()

In this way, you don't have a whole bunch of undesireable cruft floating about in your global namespace and you don't end up doing an untrusted exec. Also, execing a global statement inside a function doesn't work (as you've seen)... exec is a way to execute a string as if it were code. It isn't a way to drop dynamically created statements into the current function.

mgilson
  • 300,191
  • 65
  • 633
  • 696
0

Dictionaries, dictionaries, dictionaries. Your program should maintain control over what code gets executed, rather than letting the user construct new code dynamically.

classes = {'X': X}
instances = {}

def func1(cls):
    var = cls.lower()
    instances[var] = classes[cls]()

def func2(mode_to_set):
    instances[mode_to_set.lower()].run()

func1('X')
func2('X')

The only difference is that you don't have a global variable named x; you have a global dictionary with a key x that refers to your instance.

chepner
  • 497,756
  • 71
  • 530
  • 681