There is a lot of bad code in the world. My objective in this article is to help wxPython programmers learn how to make their applications easier to maintain and modify. It should be noted that what is in this article is not necessarily the so-called “best” way to refactor a program; instead the following is a representation of what I have learned from my own experience, with a bit of help from Robin Dunn’s book, wxPython in Action, and the wxPython community.

For illustration purposes, I created a GUI skeleton of the popular calculator program that computer science professors like to spring on unsuspecting new students. The code only creates the user interface. It will not actually do any calculations. However, I found some code on this site that should be easy to adapt for this program. I’ll leave that as an exercise for the reader. Let’s take a look at a rough, non-refactored piece of code:

import wx
 
class PyCalc(wx.App):
    def __init__(self, redirect=False, filename=None):
        wx.App.__init__(self, redirect, filename)
 
    def OnInit(self):
        # create frame here
        self.frame = wx.Frame(None, wx.ID_ANY, title="Calculator")
        panel = wx.Panel(self.frame, wx.ID_ANY)
        self.displayTxt = wx.TextCtrl(panel, wx.ID_ANY, "0", 
                                      size=(155,-1),
                                      style=wx.TE_RIGHT|wx.TE_READONLY)
        size=(35, 35)
        zeroBtn = wx.Button(panel, wx.ID_ANY, "0", size=size)
        oneBtn = wx.Button(panel, wx.ID_ANY, "1", size=size)
        twoBtn = wx.Button(panel, wx.ID_ANY, "2", size=size)
        threeBtn = wx.Button(panel, wx.ID_ANY, "3", size=size)
        fourBtn = wx.Button(panel, wx.ID_ANY, "4", size=size)
        fiveBtn = wx.Button(panel, wx.ID_ANY, "5", size=size)
        sixBtn = wx.Button(panel, wx.ID_ANY, "6", size=size)
        sevenBtn = wx.Button(panel, wx.ID_ANY, "7", size=size)
        eightBtn = wx.Button(panel, wx.ID_ANY, "8", size=size)
        nineBtn = wx.Button(panel, wx.ID_ANY, "9", size=size)
        zeroBtn.Bind(wx.EVT_BUTTON, self.method1)
        oneBtn.Bind(wx.EVT_BUTTON, self.method2)
        twoBtn.Bind(wx.EVT_BUTTON, self.method3)
        threeBtn.Bind(wx.EVT_BUTTON, self.method4)
        fourBtn.Bind(wx.EVT_BUTTON, self.method5)
        fiveBtn.Bind(wx.EVT_BUTTON, self.method6)
        sixBtn.Bind(wx.EVT_BUTTON, self.method7)
        sevenBtn.Bind(wx.EVT_BUTTON, self.method8)
        eightBtn.Bind(wx.EVT_BUTTON, self.method9)
        nineBtn.Bind(wx.EVT_BUTTON, self.method10)
        divBtn = wx.Button(panel, wx.ID_ANY, "/", size=size)
        multiBtn = wx.Button(panel, wx.ID_ANY, "*", size=size)
        subBtn = wx.Button(panel, wx.ID_ANY, "-", size=size)
        addBtn = wx.Button(panel, wx.ID_ANY, "+", size=(35,100))
        equalsBtn = wx.Button(panel, wx.ID_ANY, "Enter", size=(35,100))
        divBtn.Bind(wx.EVT_BUTTON, self.method11)
        multiBtn.Bind(wx.EVT_BUTTON, self.method12)
        addBtn.Bind(wx.EVT_BUTTON, self.method13)
        subBtn.Bind(wx.EVT_BUTTON, self.method14)
        equalsBtn.Bind(wx.EVT_BUTTON, self.method15)
        mainSizer = wx.BoxSizer(wx.VERTICAL)
        masterBtnSizer = wx.BoxSizer(wx.HORIZONTAL)
        vBtnSizer = wx.BoxSizer(wx.VERTICAL)
        numSizer  = wx.GridBagSizer(hgap=5, vgap=5)
        numSizer.Add(divBtn, pos=(0,0), flag=wx.CENTER)
        numSizer.Add(multiBtn, pos=(0,1), flag=wx.CENTER)
        numSizer.Add(subBtn, pos=(0,2), flag=wx.CENTER)
        numSizer.Add(sevenBtn, pos=(1,0), flag=wx.CENTER)
        numSizer.Add(eightBtn, pos=(1,1), flag=wx.CENTER)
        numSizer.Add(nineBtn, pos=(1,2), flag=wx.CENTER)
        numSizer.Add(fourBtn, pos=(2,0), flag=wx.CENTER)
        numSizer.Add(fiveBtn, pos=(2,1), flag=wx.CENTER)
        numSizer.Add(sixBtn, pos=(2,2), flag=wx.CENTER)
        numSizer.Add(oneBtn, pos=(3,0), flag=wx.CENTER)
        numSizer.Add(twoBtn, pos=(3,1), flag=wx.CENTER)
        numSizer.Add(threeBtn, pos=(3,2), flag=wx.CENTER)
        numSizer.Add(zeroBtn, pos=(4,1), flag=wx.CENTER)        
        vBtnSizer.Add(addBtn, 0)
        vBtnSizer.Add(equalsBtn, 0)
        masterBtnSizer.Add(numSizer, 0, wx.ALL, 5)
        masterBtnSizer.Add(vBtnSizer, 0, wx.ALL, 5)
        mainSizer.Add(self.displayTxt, 0, wx.ALL, 5)
        mainSizer.Add(masterBtnSizer)
        panel.SetSizer(mainSizer)
        mainSizer.Fit(self.frame)
        self.frame.Show()
        return True
 
    def method1(self, event):
        pass
 
    def method2(self, event):
        pass
 
    def method3(self, event):
        pass
 
    def method4(self, event):
        pass
 
    def method5(self, event):
        pass
 
    def method6(self, event):
        pass
 
    def method7(self, event):
        pass
 
    def method8(self, event):
        pass
 
    def method9(self, event):
        pass
 
    def method10(self, event):
        pass
 
    def method13(self, event):
        pass
 
    def method14(self, event):
        pass
 
    def method12(self, event):
        pass
 
    def method11(self, event):
        pass
 
    def method15(self, event):
        pass
 
 
def main():
    app = PyCalc()
    app.MainLoop()
 
if __name__ == "__main__":
    main()

I based this code on some very nasty VBA code that I have had to maintain over the last few years. This is the kind of code you’re likely to encounter from programs that auto-generate code for the programmer, such as Visual Studio or the macro builder in Microsoft Office. Notice that the functions are just numbered instead of being descriptive and that a lot of the code looks the same. When you see two or more lines of code that look the same or seem to have the same purpose, they are usually eligible for refactoring. A term for this phenomenon is “copy and paste” or spaghetti code (not to be confused with other pasta-related code euphemisms). Yes, copy and pasted code is evil! When you need to make a change, you need to find every single instance of the copied code and change it too.

On that note, let’s start refactoring this mess! I think separating the frame, the app and the panel objects makes the code easier to deal with, so that’s what we’ll do first. By looking at the widget’s parents, we see that the text control and all the buttons use the panel for their parent, so let’s put all of that into one class. I’ll also put the actual widget creation and layout into one function that can be called from the panel class’s __init__ (see below).

class MainPanel(wx.Panel):
    def __init__(self, parent):
        wx.Panel.__init__(self, parent=parent, id=wx.ID_ANY)
        self.parent = parent
        self.formula = []
        self.currentVal = "0"
        self.previousVal = "0"
        self.operator = None
        self.operatorFlag = False
        self.createAndlayoutWidgets()
 
    def createAndlayoutWidgets(self):
        mainSizer = wx.BoxSizer(wx.VERTICAL)
        masterBtnSizer = wx.BoxSizer(wx.HORIZONTAL)
        vBtnSizer = wx.BoxSizer(wx.VERTICAL)
        numSizer  = wx.GridBagSizer(hgap=5, vgap=5)
 
        self.displayTxt = wx.TextCtrl(self, wx.ID_ANY, "0", 
                                      size=(155,-1),
                                      style=wx.TE_RIGHT|wx.TE_READONLY)
 
        # number buttons
        size=(45, 45)
        zeroBtn = wx.Button(self, wx.ID_ANY, "0", size=size)
        oneBtn = wx.Button(self, wx.ID_ANY, "1", size=size)
        twoBtn = wx.Button(self, wx.ID_ANY, "2", size=size)
        threeBtn = wx.Button(self, wx.ID_ANY, "3", size=size)
        fourBtn = wx.Button(self, wx.ID_ANY, "4", size=size)
        fiveBtn = wx.Button(self, wx.ID_ANY, "5", size=size)
        sixBtn = wx.Button(self, wx.ID_ANY, "6", size=size)
        sevenBtn = wx.Button(self, wx.ID_ANY, "7", size=size)
        eightBtn = wx.Button(self, wx.ID_ANY, "8", size=size)
        nineBtn = wx.Button(self, wx.ID_ANY, "9", size=size)
 
        numBtnLst = [zeroBtn, oneBtn, twoBtn, threeBtn, fourBtn, fiveBtn,
                     sixBtn, sevenBtn, eightBtn, nineBtn]
        for button in numBtnLst:
            button.Bind(wx.EVT_BUTTON, self.onButton)
 
        # operator buttons
        divBtn = wx.Button(self, wx.ID_ANY, "/", size=size)
        multiBtn = wx.Button(self, wx.ID_ANY, "*", size=size)
        subBtn = wx.Button(self, wx.ID_ANY, "-", size=size)
        addBtn = wx.Button(self, wx.ID_ANY, "+", size=(45,100))
        equalsBtn = wx.Button(self, wx.ID_ANY, "Enter", size=(45,100))
        equalsBtn.Bind(wx.EVT_BUTTON, self.onCalculate)
 
        opBtnLst = [divBtn, multiBtn, subBtn, addBtn]
        for button in opBtnLst:
            button.Bind(wx.EVT_BUTTON, self.onOperation)
 
        numSizer.Add(divBtn, pos=(0,0), flag=wx.CENTER)
        numSizer.Add(multiBtn, pos=(0,1), flag=wx.CENTER)
        numSizer.Add(subBtn, pos=(0,2), flag=wx.CENTER)
        numSizer.Add(sevenBtn, pos=(1,0), flag=wx.CENTER)
        numSizer.Add(eightBtn, pos=(1,1), flag=wx.CENTER)
        numSizer.Add(nineBtn, pos=(1,2), flag=wx.CENTER)
        numSizer.Add(fourBtn, pos=(2,0), flag=wx.CENTER)
        numSizer.Add(fiveBtn, pos=(2,1), flag=wx.CENTER)
        numSizer.Add(sixBtn, pos=(2,2), flag=wx.CENTER)
        numSizer.Add(oneBtn, pos=(3,0), flag=wx.CENTER)
        numSizer.Add(twoBtn, pos=(3,1), flag=wx.CENTER)
        numSizer.Add(threeBtn, pos=(3,2), flag=wx.CENTER)
        numSizer.Add(zeroBtn, pos=(4,1), flag=wx.CENTER)
 
        vBtnSizer.Add(addBtn, 0)
        vBtnSizer.Add(equalsBtn, 0)
 
        masterBtnSizer.Add(numSizer, 0, wx.ALL, 5)
        masterBtnSizer.Add(vBtnSizer, 0, wx.ALL, 5)
        mainSizer.Add(self.displayTxt, 0, wx.ALL, 5)
        mainSizer.Add(masterBtnSizer)
        self.SetSizer(mainSizer)
        mainSizer.Fit(self.parent)

You’ll notice that the addition of a function and some white space has made this portion of the code look much nicer. It also enables us to take this code and put it into a separate file if we wanted to, which we could then import. This can help promote the whole MVC way of doing of things. If you were paying attention, you’ll see that I’ve bound the number buttons to one handler and the operator buttons to a different handler. Here are the methods I am employing:

def onOperation(self, event):
    """
    Add an operator to the equation
    """
    print "onOperation handler fired"
 
def onButton(self, event):
    """
    Keeps the display up to date
    """
    # Get the button object
    buttonObj = event.GetEventObject()
    # Get the label of the button object
    buttonLbl = buttonObj.GetLabel()
 
def onCalculate(self):
    """
    Calculate the total
    """
    print 'in onCalculate'

I stuck some code in the onButton event handler so that you could see how to get a handle to the button object that called it. Otherwise, this method doesn’t really do anything. The other methods are just stubs. Now let’s look at the frame and app object code:

class PyCalcFrame(wx.Frame):
    def __init__(self):
        wx.Frame.__init__(self, parent=None, id=wx.ID_ANY, 
                          title="Calculator")
        panel = MainPanel(self)
 
class PyCalc(wx.App):
    def __init__(self, redirect=False, filename=None):
        wx.App.__init__(self, redirect, filename)
 
    def OnInit(self):
        # create frame here
        frame = PyCalcFrame()
        frame.Show()
        return True
 
def main():
    app = PyCalc()
    app.MainLoop()
 
if __name__ == "__main__":
    main()

As you can see, they are very short and to the point. If you’re in a hurry, then this little bit of refactoring might be enough for you. However, I think we can do better. Scroll back up and take a second look at that refactored panel class. See how there are over a dozen button creation lines that are basically the same? There are also a lot of “sizer.Add” lines in there too. Those will be our next target!

On the wxPython mailing list this last spring (2009), there was a large discussion on this topic. I saw lots of interesting solutions, but the one that was bandied about the most was the creation of some kind of widget building method. That’s what I will show you how to do. Here is my limited version:

def onWidgetSetup(self, widget, event, handler, sizer, pos=None, flags=[]):
    """
    Accepts a widget, the widget's default event and its handler,
    the sizer for the widget, the position of the widget inside 
    the sizer (if applicable) and the sizer flags (if applicable)
    """
    widget.Bind(event, handler)        
    if not pos:
        sizer.Add(widget, 0, wx.ALL, 5)
    elif pos and flags:
        sizer.Add(widget, pos=pos, flag=wx.CENTER) 
    else:
        sizer.Add(widget, pos=pos)
 
    return widget

This is a pretty simple bit of code that takes a widget, binds it to an event and adds the resultant combination to a sizer. This script could be expanded to do additional bindings, nest sizers, set fonts and much more. Use your imagination. Now we can take a look at how this has changed the panel class code:

class MainPanel(wx.Panel):
    def __init__(self, parent):
        wx.Panel.__init__(self, parent=parent, id=wx.ID_ANY)
        self.parent = parent
        self.formula = []
        self.currentVal = "0"
        self.previousVal = "0"
        self.operator = None
        self.operatorFlag = False
        self.createAndlayoutWidgets()
 
    def createAndlayoutWidgets(self):
        mainSizer = wx.BoxSizer(wx.VERTICAL)
        masterBtnSizer = wx.BoxSizer(wx.HORIZONTAL)
        vBtnSizer = wx.BoxSizer(wx.VERTICAL)
        numSizer  = wx.GridBagSizer(hgap=5, vgap=5)
 
        self.displayTxt = wx.TextCtrl(self, wx.ID_ANY, "0", 
                                      size=(155,-1),
                                      style=wx.TE_RIGHT|wx.TE_READONLY)
        # number buttons
        size=(45, 45)
        zeroBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "0", size=size),
                                     wx.EVT_BUTTON, self.onButton, numSizer,
                                     pos=(4,1))
        oneBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "1", size=size),
                                    wx.EVT_BUTTON, self.onButton, numSizer,
                                    pos=(3,0))
        twoBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "2", size=size),
                                    wx.EVT_BUTTON, self.onButton, numSizer,
                                    pos=(3,1))
        threeBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "3", size=size),
                                      wx.EVT_BUTTON, self.onButton, numSizer,
                                      pos=(3,2))
        fourBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "4", size=size),
                                     wx.EVT_BUTTON, self.onButton, numSizer,
                                     pos=(2,0))
        fiveBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "5", size=size),
                                     wx.EVT_BUTTON, self.onButton, numSizer,
                                     pos=(2,1))
        sixBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "6", size=size),
                                    wx.EVT_BUTTON, self.onButton, numSizer,
                                    pos=(2,2))
        sevenBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "7", size=size),
                                      wx.EVT_BUTTON, self.onButton, numSizer,
                                      pos=(1,0))
        eightBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "8", size=size),
                                      wx.EVT_BUTTON, self.onButton, numSizer,
                                      pos=(1,1))
        nineBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "9", size=size),
                                     wx.EVT_BUTTON, self.onButton, numSizer,
                                     pos=(1,2))
        # operator buttons
        divBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "/", size=size),
                                    wx.EVT_BUTTON, self.onOperation, numSizer,
                                    pos=(0,0), flags=wx.CENTER)
        multiBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "*", size=size),
                                      wx.EVT_BUTTON, self.onOperation, numSizer,
                                      pos=(0,1), flags=wx.CENTER)
        subBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "-", size=size),
                                    wx.EVT_BUTTON, self.onOperation, numSizer,
                                    pos=(0,2), flags=wx.CENTER)
        addBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "+", size=(45,100)),
                                    wx.EVT_BUTTON, self.onOperation, vBtnSizer)
        equalsBtn = self.onWidgetSetup(wx.Button(self, wx.ID_ANY, "Enter", size=(45,100)),
                                       wx.EVT_BUTTON, self.onCalculate, vBtnSizer)
        masterBtnSizer.Add(numSizer, 0, wx.ALL, 5)
        masterBtnSizer.Add(vBtnSizer, 0, wx.ALL, 5)
        mainSizer.Add(self.displayTxt, 0, wx.ALL, 5)
        mainSizer.Add(masterBtnSizer)
        self.SetSizer(mainSizer)
        mainSizer.Fit(self.parent)
 
    def onWidgetSetup(self, widget, event, handler, sizer, pos=None, flags=[]):
        """
        Accepts a widget, the widget's default event and its handler,
        the sizer for the widget, the position of the widget inside 
        the sizer (if applicable) and the sizer flags (if applicable)
        """
        widget.Bind(event, handler)        
        if not pos:
            sizer.Add(widget, 0, wx.ALL, 5)
        elif pos and flags:
            sizer.Add(widget, pos=pos, flag=wx.CENTER) 
        else:
            sizer.Add(widget, pos=pos)
 
        return widget

The best part about this code is that it puts all the button creation stuff into a method, so we don’t have to write “wx.Button()” over and over again. This iteration also removed most of the “sizer.Add” calls. Of course, in its place, we have a lot of “self.onWidgetSetup()” method calls instead. It looks like this can still be refactored, but how!? For my next trick, I looked through the Refactoring section in Robin Dunn’s book and concluded that his ideas were worth a try. (He is the creator of wxPython, after all.)

In his book, he has a button builder method that looks similar to my widget builder one, although his is much simpler. He also has a method that just returns button data. I’ve taken those ideas and adapted them for this program, as you can see in my final panel code:

class MainPanel(wx.Panel):
    def __init__(self, parent):
        wx.Panel.__init__(self, parent=parent, id=wx.ID_ANY)
        self.parent = parent
        self.formula = []
        self.currentVal = "0"
        self.previousVal = "0"
        self.operator = None
        self.operatorFlag = False
        self.createDisplay()
 
    def createDisplay(self):
        """
        Create the calculator display
        """
        mainSizer = wx.BoxSizer(wx.VERTICAL)
        masterBtnSizer = wx.BoxSizer(wx.HORIZONTAL)
        vBtnSizer = wx.BoxSizer(wx.VERTICAL)
        numSizer  = wx.GridBagSizer(hgap=5, vgap=5)
 
        self.displayTxt = wx.TextCtrl(self, wx.ID_ANY, "0", 
                                      size=(155,-1),
                                      style=wx.TE_RIGHT|wx.TE_READONLY)
 
        for eachLabel, eachSize, eachHandler, eachPos in self.buttonData():
            button = self.buildButton(eachLabel, eachSize, eachHandler)
            if eachPos:
                numSizer.Add(button, pos=eachPos, flag=wx.CENTER) 
            else:
                vBtnSizer.Add(button)
 
        masterBtnSizer.Add(numSizer, 0, wx.ALL, 5)
        masterBtnSizer.Add(vBtnSizer, 0, wx.ALL, 5)
        mainSizer.Add(self.displayTxt, 0, wx.ALL, 5)
        mainSizer.Add(masterBtnSizer)
        self.SetSizer(mainSizer)
        mainSizer.Fit(self.parent)
 
    def buttonData(self):
        size=(45, 45)
        return (("0", size, self.onButton, (4,1)), 
                ("1", size, self.onButton, (3,0)),
                ("2", size, self.onButton, (3,1)), 
                ("3", size, self.onButton, (3,2)),
                ("4", size, self.onButton, (2,0)), 
                ("5", size, self.onButton, (2,1)),
                ("6", size, self.onButton, (2,2)), 
                ("7", size, self.onButton, (1,0)),
                ("8", size, self.onButton, (1,1)), 
                ("9", size, self.onButton, (1,2)),
                ("/", size, self.onOperation, (0,0)), 
                ("*", size, self.onOperation, (0,1)),
                ("-", size, self.onOperation, (0,2)),
                ("+", (45,100), self.onOperation, None),
                ("Enter", (45,100), self.onCalculate, None))
 
    def buildButton(self, label, size, handler):
        """
        Builds a button and binds it to an event handler.
        Returns the button object
        """
        button = wx.Button(self, wx.ID_ANY, label, size=size)
        self.Bind(wx.EVT_BUTTON, handler, button)
        return button

At this point, we should step back and see what this has gained us. The original code was 132 lines, the first refactor bumped the line count down to 128, the second increased the count to 144 and this last one took us back down to 120. The cynical might say that all we’ve saved is 12 lines of code. I disagree. What we end up with (whether or not it’s more code than the original) is a much easier-to-maintain code base. This can be modified and kept clean much easier than the original.

I hope this post has helped you see how refactoring your code into classes and methods can make your programs more readable, easier to maintain — and share with other contributors, shame-free!

Downloads

Further Reading

Print Friendly