4

I'm learning Python on my R.Pi and I've hit a small snag. It seems to me that the following code would leave the "inputchecker" function open in memory while it routes back to the "getinput" function.

Is this bad code? Should it be done very differently?

def getinput(i):    
    if i == 1:
        first = input("Would you like A or B? ")
        inputchecker(1, first)
    elif i == 2:
        second = input("Would you like C or D? ")
        inputchecker(2, second)

def inputchecker(n, userinput):    
    def _tryagain_(n):
        usage(n)
        getinput(n)        
    if n == 1:
        if userinput in ("A", "B"):
            print("You chose wisely.")
            getinput(2)
        else:
            _tryagain_(n)
    elif n == 2:
        if userinput in ("C", "D"):
            print("You chose wisely.")
        else:
            _tryagain_(n)

def usage(u):
    if u == 1:
        print("Usage: Just A or B please.") 
    if u == 2:
        print("Usage: Just C or D please.") 


getinput(1)
Brendan Abel
  • 35,343
  • 14
  • 88
  • 118
Benjo
  • 147
  • 1
  • 5
  • It isn't exactly pythonic, as it can cause problems with unforeseen true if statements, and this isn't limited in python only. All languages support this "hack". It's best to use a while loop, unless you use the while loop often enough to put it into a function. – WorkingRobot Nov 22 '16 at 18:21

3 Answers3

2

No, the name getinput in the nested function doesn't create a reference. It is looked up each time _tryagain_ is called, because it is a global. Not that this matters, as a module is cleared as a whole when Python exits, there is no real chance for a memory leak here.

However, you are using recursion to ask users for input, and your code is hard to follow. Use a simple loop instead, see Asking the user for input until they give a valid response.

Community
  • 1
  • 1
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
1

Having two functions infinitely call each other isn't really the best control flow. It would be better with a while loop

def getinput(i):
    while i:    
        if i == 1:
            first = input("Would you like A or B? ")
            i = inputchecker(1, first)
        elif i == 2:
            second = input("Would you like C or D? ")
            i = inputchecker(2, second)

def inputchecker(n, userinput):          
    if n == 1:
        if userinput in ("A", "B"):
            print("You chose wisely.")
            return 2
        else:
            getusage(i)
            return i
    elif n == 2:
        if userinput in ("C", "D"):
            print("You chose wisely.")
        else:
            getusage(i)
            return i

It would probably be even better if you simplified it into a single function. There's no reason it needs to be split up.

Brendan Abel
  • 35,343
  • 14
  • 88
  • 118
0

I would certainly avoid the recursive calls. Also, I would make make the validation function return a boolean instead of the number of the next question. Since you ask the questions anyway in sequence, that seems to only complicate it for the reader of your code.

Also I would let the validation always return something -- you never know: the first argument might be wrong as well:

def getinput():
    valid = False
    while not valid:
        first = input("Would you like A or B? ")
        valid = inputIsValid(1, first)
    valid = False
    while not valid:
        second = input("Would you like C or D? ")
        valid = inputIsValid(2, second)
    return [first, second]

def inputIsValid(n, userinput):    
    valid = False
    if n == 1:
        valid = userinput in ("A", "B")
    elif n == 2:
        valid = userinput in ("C", "D")
    if valid:
        print("You chose wisely.")
    else:
        usage(n)
    return valid

def usage(u):
    if u == 1:
        print("Usage: Just A or B please.") 
    elif u == 2:
        print("Usage: Just C or D please.") 

getinput() 
trincot
  • 317,000
  • 35
  • 244
  • 286