Code style - 'hiding' functions inside other functions

Question

Recently I've been doing things like this:

import Tkinter
class C(object):
    def __init__(self):
        self.root = Tkinter.Tk()
        def f():
            print 'hello'
        self.button = Tkinter.Button(master=self.root, command=f, text='say hello')

as opposed to something like this:

import Tkinter
class C(object):
    def __init__(self):
        self.root = Tkinter.Tk()
        self.button = Tkinter.Button(master=self.root, command=self.f, text='say hello')
    def f(self):
        print 'hello'

The question isn't specific to Tkinter, but it's a good example. The function f is only used as the callback for the button, so I've chosen to define it inside __init__. That way, only code inside __init__ even knows about f's existence - outer scopes don't start getting cluttered up with names, and the user doesn't need to care about a load of methods designed to be internal.

My question is: is this regarded as good style? I'm worrying because I've got a GUI class with quite a lot of buttons - __init__ is starting to look very long, with a lot of local function definitions. Is there a more appropriate alternative that I should be using?

Answer 1

The typical way to do something like this in the context of Tkinter is to use a lambda function.

self.button = Tkinter.Button(master=self.root, 
                             command=lambda:sys.stdout.write("Hello!\n"),
                             text='say hello')

At it's core, this is really the same as your first example though, so if you prefer the first way -- Go with it. I think that it is generally not idiomatic to create a new method in this case (unless you actually need the instance to be passed to the callback -- in which case, you should do it the second way).

There are two things to worry about here. The first is readability. If __init__ gets too cluttered to read, then you have a problem. You can move those functions to the module level (prefixed with an _ to keep them from importing in the from module import * context) and use lambda to bind local variables as arguments to those functions if necessary.

If __init__ isn't getting cluttered, then feel free to put the functions in there. Having the functions in there does mean that a new function is created every time a new instance is created (same with lambda) which I suppose could be wasteful as far as memory is concerned, but it shouldn't be that big of a deal (especially in a gui).

The second thing to worry about is namespace cluttering. However, if your module is so big that moving those local functions to module level functions creates a namespace issue, then your module is too big to begin with. As long as you prefix the functions with underscores (see suggestion above), you shouldn't have namespace issues importing this module in other ones either.

Answer 2

In this case it's harmless. On the whole, though, I would avoid it for two reasons:

1) The definition of the callbacks as a member of the class is somewhat more readable (especially when you're using pydoc to investigate)

2) Creating the function inside another introduces more closures (variables inherited from the calling context).