Troubles with global variables

[Jeff Shannon]

Jeremy Whetzel wrote:

> enc = 'gogo'
> br = '128'
>
> eo = [['E',') Encoder: ','chooseenc()',enc],
>       ['B',') Bitrate: ','choosebr()',br]]
>
> def menumake(menu):
>   global enc,br
>   os.system('clear')
>   for x in menu:
>     print x[0] + x[1] + x[3]

As someone else already noted, here you're printing constant strings.
You're changing enc and br, but not eo/menu.


>   for y in menu:
>     if choice == y[0]:
>       exec y[2]

Instead of saving the name of your function as a string, and then exec'ing
that string, it would be easier (and safer) to save a reference to the
function and call it directly.  In other words:

eo = [['E', 'Encoder', chooseenc, enc],
          ['B', 'Bitrate', choosebr, br]]
def menumake(menu):
    ....
    for item in menu:
        if choice == item[0]:
            item[2]()

I'd also recommend that, instead of using a list of lists for this, that
you use dictionaries, with the option character as the key and a tuple of
(option, description, value) as the values.  For instance.....

menudict = { 'E' :  ('E', 'Encoder', 'gogo' ),
               'B' : ('B', 'Bitrate', '128' ),
               'Q' : ('Q', 'Quit', '' )     }


def showmenu(menu):
    while 1:    #use a loop, rather than recursively calling itself
        os.system('clear')
        for item in menu.values():  #iterate over the contents of the
dictionary
            print "  %s )  %15s   %s" % item  # using string formatting for
the display
        print
        choice = string.upper(raw_input('Enter Choice: '))
        print

        if choice == 'Q':
            break
        elif menu.has_key(choice):
            prompt = 'Enter the new %s: ' % menu[choice][1]
            menu[choice][2] =  raw_input(prompt)
        else:
            print "That is not a valid option."

if __name__ == '__main__':
    showmenu(menudict)

------------------
A few notes about this.

Instead of having menumake call itself recursively, as you've done, I've
made it run in an infinite loop (until told to quit).  Recursion will
eventually run into stack-depth limits and such--it's useful when you're
subdividing a problem into smaller chunks, but when you're repeating the
same thing, a loop is usually a better bet.

Note that I never used any global statement, and that my function never
actually references the global name menudict.  Since dictionaries are
mutable objects, I can access the same dictionary through any name that's
bound to it.  The act of passing the dictionary as a function parameter
binds a new name (menu) to it, and I use that to modify the object.  This
is a bit different than assigning a new value to a name, as you were trying
to do in your code.

I've also used tuples as the values in the dictionary, to allow easy string
formatting for the printout.  The only reason to include the option key
again in the tuple, is to simplify that formatting.  I could have left that
out, and had the printout loop instead be:

    for key, value in menu.items():
        print "  %s )  %15s %s" % (key, value[0], value[1])

but the way I did it above seems more aesthetically pleasing to me.

I've also done away with the helper functions chooseenc() and choosebr()
entirely.  Switching to using a dictionary to store our data makes changing
things simple enough that these helper functions are not necessary.
However, if you were doing something more complicated than just changing a
single quantity in your menu item, it would be easy enough to have the
dictionary value also store a function reference, and to call that
reference as I demonstrated near the top of this post.

I hope that this example will help you to see some of the possibilities of
Python, and give you an idea of how to do things in a slightly more
Pythonic (imho) way.  :)

Jeff Shannon
Technician/Programmer
Credit International