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
Not very convincing…the main problem is still there : the hard-coded config value in the code.
The Gtk workflow with glade looks much more sensible in this case (it does have other problems, tho).
Not very convincing…the main problem is still there : the hard-coded config value in the code.
The Gtk workflow with glade looks much more sensible in this case (it does have other problems, tho).
Good article. I usually use wxGlade for GUI building, and it will output really good code (definitely not similar to what you’re showing in the first example)
Good article. I usually use wxGlade for GUI building, and it will output really good code (definitely not similar to what you’re showing in the first example)
@ Tiberiu,
I didn’t think that Python tools would do that, but I haven’t used the code generators much since getting bit by Microsoft’s. Thanks for the feedback!
@ Lionel,
I assume you’re referring to the hard coded values for the button labels or maybe the position of the widgets? There are other sizers I could have used that wouldn’t require the position to be explicit. I just used the one I thought most new wxPython programmers would use.
– Mike
@ Lionel,
I assume you’re referring to the hard coded values for the button labels or maybe the position of the widgets? There are other sizers I could have used that wouldn’t require the position to be explicit. I just used the one I thought most new wxPython programmers would use.
– Mike
For the gross structure of a new wxPython program I bang it out in wxGlade. It makes it fairly painless to get most of the widgets onto the frame into whatever sizers. It does have it’s limitations though, with temperamental support for custom classes and controls (unable to find them when attempting to preview, etc). Even after diving into my own subclassed panels or whatever, I’ll make the class then tell wxGlade about it so that way the code generation still works all throughout in case I want to do some basic restructuring.
Binding events I do myself for whatever reason (probably because the first few examples I saw did it that way)
wxGlade’s structure it generates is pretty basic; easy enough to take wherever you like. The stuff in the autogen “# begin wxGlade” to “# end wxGlade” sections isn’t exactly pretty, but who cares; you’re not supposed to edit it directly.
For the gross structure of a new wxPython program I bang it out in wxGlade. It makes it fairly painless to get most of the widgets onto the frame into whatever sizers. It does have it’s limitations though, with temperamental support for custom classes and controls (unable to find them when attempting to preview, etc). Even after diving into my own subclassed panels or whatever, I’ll make the class then tell wxGlade about it so that way the code generation still works all throughout in case I want to do some basic restructuring.
Binding events I do myself for whatever reason (probably because the first few examples I saw did it that way)
wxGlade’s structure it generates is pretty basic; easy enough to take wherever you like. The stuff in the autogen “# begin wxGlade” to “# end wxGlade” sections isn’t exactly pretty, but who cares; you’re not supposed to edit it directly.
Nice job, Mike. I really want to make refactoring more of a priority now that I am not quite so shaky in wxPython and have some old code that was written when I was. 😀
FWIW, Boa Constructor generates all the GUI code in a section that falls under the function called self._init_ctrls(). Within that, there is then a call to self._init_sizers(). So all the GUI code (and some utilities code, like wxTimer) is taken care of in one standardized section near the start of the code, with line breaks separating things nicely.
Everything else I add myself, starting under the main __init__(). Of course, that’s where I have to apply the refactoring–my own code.
That’s really a good advice. The students need to know they are making a choice for their lives. It’s an important step to start building their careers.
I use quite often something similar to *buttonData* but to build recursive (main | popup) menus
Thnx for sharing !
I use quite often something similar to *buttonData* but to build recursive (main | popup) menus
Thnx for sharing !