Are we setting bad examples by placing code logic into event handlers?
Steven Sproat
placing our "business logic" directly into the event handlers, which
violates all manners of good OO design principles. Ideally the event
handlers would delegate the method calls to the appropriate objects,
passing any Event attributes as parameters.
The way we go about teaching things ends up with quite "spaghetti code"
due to classes storing references to each other - so you end up calling
self.gui.panel.textbox.GetValue() from within some other class which is
all sorts of wrong according to The Law of Demeter.
What'd you guys think about this? I'm trying to change my application so
any event handling code is at maximum 3 lines long, and performs very
little logic. The idea is that the entire system can be unit tested much
easier as there will be no explicit dependency on the GUI.
--
Steven Sproat, BSc
http://whyteboard.org/
Mark
call it from the LEFT_DOWN handler in one place and the LEFT_UP
handler in another.
Really the 3 line event handler thing is not at all related to OO
design and will actually cause you problems. I just ported my desktop
app over to CherryPy to create a web app. Do I really need an OnEnter
or OnMouse handler in my data class? The web app will never use it -
the data will be entered in a completely different way. So the
displaying and manipulating for display is left to my GUI objects.
I use a lot of wx.GetApp's like so:
wx.GetApp().frame.status_bar.SetStatusText(u"",0)
Is that a problem? Theres nothing wrong with classes being linked to
each other.
My app has a frame with a notebook in it and then every page in the
notebook is a separate panel derived class. About 80% of my code is
related to displaying the data that is contained in my data classes
and I bind my events to those panels so there is no one else that
needs to handle those events. The web app uses the common data
classes, but data is input and displayed completely differently due to
the differing medium.
It sounds like you have a wxPython GUI derived class and want to
create an accompanying still GUI specific class which doesn't make
sense to me. Events are GUI events your data classes should have no
clue how to handle GUI events. For unit testing it doesn't matter if
you dummy up an event argument of a couple string args.
What does your code look like?
Mark
C M
> Everywhere we're teaching wx (the wiki, demo, list etc) we're always
> placing our "business logic" directly into the event handlers, which
> violates all manners of good OO design principles. Ideally the event
> handlers would delegate the method calls to the appropriate objects,
> passing any Event attributes as parameters.
I didn't do this when I was first learning, but now I try to make my
event handlers have just one call to a function (usually within that
class) that does whatever is needed. This way, the logic and the GUI
element are less dependent on each other. I can change my mind from a
regular wxButton to a wxBitmapButton and not have to remember to move
the logic code under the event handler for the BitmapButton. It also
allows me to bind different GUI objects to the same logic, in case I
want some redundancy (do something by pushing a button or selecting a
menu item). It also allows me to give more descriptive names to the
event handlers and the functions, so I can scan in (in my case) Boa's
Explore tab and see I have an event handler called, e.g.,
OnGetTemperatureButton() and then I have a function called
GetTemperature() and it is very clear what they both do. If I had the
temperature-getting code under the event handler only, I would have to
repeat it elsewhere to use it for non-GUI use.
> The way we go about teaching things ends up with quite "spaghetti code"
> due to classes storing references to each other - so you end up calling
> self.gui.panel.textbox.GetValue() from within some other class which is
> all sorts of wrong according to The Law of Demeter.
I'm guilty of some of that, but, based on advice here, I have tried to
incorporate pubsub into my efforts to reduce it. I actually tried to
refactor what I was working on into a MVC pattern and it was rather a
pain and I just gave up and now I try to think in terms of modules
that do both logic and GUI as long as they are tightly related--this
way that module takes care of everything related to what is needed in
that part of the application. Is that "ravioli code"? Maybe. So
far, I am going with it.
Che
Mike Driscoll
On Jul 12, 12:12 am, C M <cmpyt...@gmail.com> wrote:
so easy to break that...instead, I use much of what Che is talking
about. Pubsub is great! I've also messed with PostEvent, although I
don't like that method as much. Alas, my event handlers aren't usually
super small, but I could put most of my handler stuff in a different
file if I wanted to.
-------------------
Mike Driscoll
Blog: http://blog.pythonlibrary.org
Robin Dunn
> Everywhere we're teaching wx (the wiki, demo, list etc) we're always
> placing our "business logic" directly into the event handlers, which
> violates all manners of good OO design principles.
I've thought about this somewhat in the past, and have considered the
merits of teaching better application design via the samples and demos,
but I found a few reasons not to do so:
* KISS. I think that the samples that people use to learn should be as
simple as possible, and show the bare minimum needed to learn the aspect
of wx being demoed. I remember when I was learning Java and how
annoying it was to have to learn so much of the standard framework and
all of the "big picture" stuff behind it in order to do anything at all,
even for a Hello World.
* Most OO design frameworks that would scale up well for a large
application would add a lot of complex cruft to a simple sample
application that would get in the way of teaching about the gui elements
themselves. I don't think a newbie should be required to learn about
something like pubsub or even wx.CallAfter just to be able to learn how
to use a wx.Button. Imposing "Do it this way, you'll thank me later"
types of constructs in simple examples just adds confusion and detracts
from the purpose of the sample.
* Switching the samples over to some better design pattern would mean
determining what exactly should be the wxPython-blessed pattern, and
then eventually code would show up in the core and/or in the library
that assumes or even requires that that pattern be used throughout the
application. Something else I've banged my head against in the past has
been when some library or tool designer has decided that their way of
doing things is better than any other way and so no other way is
allowed, or when their way imposes arbitrary limitations on what the
tool can do. One thing I've learned is that anytime there is an
application large enough that it really needs to follow a good design
pattern then what is best for that application and/or developer is
probably different than most others. I've used different patterns for
every large project I've worked on (sometimes even at the same time,)
and I think that maintaining the freedom for developers to implement
what they see as the best solution for their project is very important
because no matter how perfect what we pick is, somebody will always want
something different.
--
Robin Dunn
Software Craftsman
http://wxPython.org
Mark
> > OnGetTemperatureButton() and then I have a function called
> > GetTemperature() and it is very clear what they both do. If I had the
> > temperature-getting code under the event handler only, I would have to
> > repeat it elsewhere to use it for non-GUI use.
case where it makes sense to have 2 functions here. There is no non
GUI use of a wx.Panel derived class that is catching button events.
> I almost never do the "self.gui.panel.textbox.GetValue()" because it's
> so easy to break that...instead, I use much of what Che is talking
> about. Pubsub is great! I've also messed with PostEvent, although I
> don't like that method as much. Alas, my event handlers aren't usually
> super small, but I could put most of my handler stuff in a different
> file if I wanted to.
handling stuff in a separate file. All separating the code does is
allow for more bugs. The majority of my event handlers are
manipulating the data, but they are manipulating it for display
purposes which is GUI specific. When I want to update the data I call
wx.GetApp().data.update(**data). When the user moves around the GUI
the GUI classes call wx.GetApp().data.IsModified? and updates
themselves accordingly. Putting pubsub code into my data class is
adding code that is specific to this app which is against the
principle of reuse.
I'm an MBA who picked up wxPython so I could start my business in my
spare time so I surely don't have the background anyone else does
here, but surely the point is code reuse and getting rid of easily to
bug code.
Heres an MVC tutorial:
http://wiki.wxpython.org/ModelViewController/
I think the argument for pubsub here is wrong in that there was no
problem originally as the view certainly knows when it needs to query
the model - it makes no sense to add code to your data classes to
force them to update the GUI when that information is not even needed
if the user is looking at a different page.
Mark
Kevin Ollivier
The problem with using small samples for MVC is that it's hard to illustrate with a 100 line app exactly how one benefits from using MVC. (The sample you link to more just shows how you would go about MVC with wx.) MVC is all about ease of refactoring - you can quickly adjust to changes in your GUI layout, your business logic, etc. without having to re-write lots of your code just to update references from one object to another. Until you've had the pleasure of going through a bout of painful refactoring of code lacking MVC design, though, the light bulb won't really go off for you.
Say you did what you would naturally do, and put self.myMoney inside of View and bound the AddMoney and RemoveMoney handlers inside of the View's __init__ method, and had those methods directly manipulate self.myMoney. Then, after you've done that, users (or your clients / bosses) ask for a GraphView of the money, and a TimelineView and, oh, and they want a web-based API so that they can integrate it with Google Docs, and of course, they want it to be easy to install on servers which probably don't have wx installed. Does the initial design still seem like it will be up to the task? While in this very simple case it'd be easy to refactor, in a real world set up it would take considerably more effort. It's a LOT less painful to do the small extra effort of designing with MVC up front then it is to totally restructure your code to add it later. I know this because when I started wx coding I avoided both sizers and MVC, and I had to spend a lot of time undoing my old approach and re-writing a more robust one for the same code.
Until you come across a scenario like the above where the need for separation becomes obvious, MVC sounds like much ado about nothing. It's similar to sizers in that way. If you're coding on one platform and have relatively static resolutions to support, sizers are just an extra hassle, and pixel-perfect layout is faster and less to learn. Move your app to several different platforms and/or display resolutions, though, and you'll consider them a godsend and, if you have a decently large app that was written WITHOUT sizers, you'll kick yourself for not just taking the plunge and learning them.
The question Steven poses is, is it better to just let users learn how to do MVC design the hard way, or should wx samples and docs point them in the right direction? I tend to agree with Steven in this regards. As with sizers, I think it's okay to promote an approach where you may end up saying "you may not realize the need for this right now, but one day you'll find it makes a hard problem simple, and you'll really be glad you learned it". I think it would also indirectly lead to more users writing and contributing mixins, making adding advanced functionality quicker and easier.
We certainly should not force users to take any particular approach, but I think promoting good design is another issue entirely and I think users could really stand to benefit from seeing a different and more robust approach. Lots of people are not particular about how they code their app - look at how much wx code starts as straight copy and paste off of samples. Perhaps some users will reject the idea, heck I don't like the current wx approach now so you can never make everyone happy, but if we don't even show a more robust way in any real sense (except in an isolated sample here, or a wiki entry there), then we risk wx users who write major apps coming out with the impression that wx isn't really up to the job of creating robust and large-scale applications. And those are the people we need to build a community of long-term supporters.
Regards,
Kevin
> Mark
>
> --
> To unsubscribe, send email to wxPython-user...@googlegroups.com
> or visit http://groups.google.com/group/wxPython-users?hl=en
Steven Sproat
> Hi Mark,
>
> On Jul 12, 2010, at 1:12 PM, Mark wrote:
>
>
>
> The problem with using small samples for MVC is that it's hard to illustrate with a 100 line app exactly how one benefits from using MVC. (The sample you link to more just shows how you would go about MVC with wx.) MVC is all about ease of refactoring - you can quickly adjust to changes in your GUI layout, your business logic, etc. without having to re-write lots of your code just to update references from one object to another. Until you've had the pleasure of going through a bout of painful refactoring of code lacking MVC design, though, the light bulb won't really go off for you.
>
> Say you did what you would naturally do, and put self.myMoney inside of View and bound the AddMoney and RemoveMoney handlers inside of the View's __init__ method, and had those methods directly manipulate self.myMoney. Then, after you've done that, users (or your clients / bosses) ask for a GraphView of the money, and a TimelineView and, oh, and they want a web-based API so that they can integrate it with Google Docs, and of course, they want it to be easy to install on servers which probably don't have wx installed. Does the initial design still seem like it will be up to the task? While in this very simple case it'd be easy to refactor, in a real world set up it would take considerably more effort. It's a LOT less painful to do the small extra effort of designing with MVC up front then it is to totally restructure your code to add it later. I know this because when I started wx coding I avoided both sizers and MVC, and I had to spend a lot of time undoing my old approach and re-writing a more robust one for the same code.
>
> Until you come across a scenario like the above where the need for separation becomes obvious, MVC sounds like much ado about nothing. It's similar to sizers in that way. If you're coding on one platform and have relatively static resolutions to support, sizers are just an extra hassle, and pixel-perfect layout is faster and less to learn. Move your app to several different platforms and/or display resolutions, though, and you'll consider them a godsend and, if you have a decently large app that was written WITHOUT sizers, you'll kick yourself for not just taking the plunge and learning them.
>
> The question Steven poses is, is it better to just let users learn how to do MVC design the hard way, or should wx samples and docs point them in the right direction? I tend to agree with Steven in this regards. As with sizers, I think it's okay to promote an approach where you may end up saying "you may not realize the need for this right now, but one day you'll find it makes a hard problem simple, and you'll really be glad you learned it". I think it would also indirectly lead to more users writing and contributing mixins, making adding advanced functionality quicker and easier.
>
> We certainly should not force users to take any particular approach, but I think promoting good design is another issue entirely and I think users could really stand to benefit from seeing a different and more robust approach. Lots of people are not particular about how they code their app - look at how much wx code starts as straight copy and paste off of samples. Perhaps some users will reject the idea, heck I don't like the current wx approach now so you can never make everyone happy, but if we don't even show a more robust way in any real sense (except in an isolated sample here, or a wiki entry there), then we risk wx users who write major apps coming out with the impression that wx isn't really up to the job of creating robust and large-scale applications. And those are the people we need to build a community of long-term supporters.
>
> Regards,
>
> Kevin
>
>
have. I've been pondering how to reply and convey my points clearly.
Yes, testing and refactoring are the main motives for this discussion.
By showing users "okay this is how we do it" we do show how to implement
the core logic easily, but these tend to develop into full-blown
applications where everything can just be a mess (I say this from
personal experience!)
There are many OO principles that discuss best practices and how to
miminise tight coupling between your modules. When there are lots of
code logic inside your event handlers, how can you unit test these to
ensure the code is behaving as expected? You'd have to create many
stub/mock code to represent your wx objects, as they are tightly coupled
to them. So you need to create mock Event objects for instance that
return dummy values at given times.
I agree with Robin and Kevin that the samples are well-suited towards
coupled code as it clarifies the code's intent, which is needed for
small samples for users to learn from. Perhaps we should create a wiki
section promoting code cleanliness and good practices. I find that
pubsub is great for this - while the pubsub "sendMessage" is essentially
equivalent to a direct method call, the lack of any explicit
dependencies between the modules makes for easier testing. If I use
"gui.change_tab" as my topic name, it does map to a method call, but
multiple message listeners can subscribe to that topic and handle the
change as appropriate. One message passed may end up starting a chain of
50 method calls inside various different modules.
This also means that there are less bugs - if a change is made, then
instead of tracking down all calls to gui.change_tab, we can just change
the relevant topic listeners. I've had many bugs due to making a new
change/feature bug forgetting to apply the change to one obscure place.
http://en.wikipedia.org/wiki/Law_Of_Demeter - The "Law of
Demeter/Principle of least knowledge" states how objects should have
small knowledge of their corresponding modules. for example, if module A
has an association with module B, and module B is associated to C, A
should never access C directly through B. Instead, it should call
methods on B that in turn accesses C.
This does lead to more delegate methods in your classes but the end
result is the client code that uses these classes has a nicer interface
to work with.
..phew! long posd!
Christopher Barker
> Everywhere we're teaching wx (the wiki, demo, list etc) we're always
> placing our "business logic" directly into the event handlers, which
> violates all manners of good OO design principles.
> so you end up calling
> self.gui.panel.textbox.GetValue() from within some other class which is
> all sorts of wrong according to The Law of Demeter.
Not if you follow this:
http://wiki.wxpython.org/wxPython%20Style%20Guide
But yes, you are right.
It's a real pedagogical challenge - you want to be able to show folks
how to use a given control without a whole lot of extra code, but then
the natural thing for a newbie to do is use that code as a model, and,
you're quite right, end up with spaghetti.
I like mid-size, rather than tiny, examples, but it's really hard to
show good code structure in a small program, no matter how you do it.
-Chris
--
Christopher Barker, Ph.D.
Oceanographer
Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception
Kevin Ollivier
Wow, sorry, I thought I sent this a long time ago but when I checked I found it was still in my drafts folder.
On Jul 12, 2010, at 3:34 PM, Christopher Barker wrote:
> Steven Sproat wrote:
>> Everywhere we're teaching wx (the wiki, demo, list etc) we're always
>> placing our "business logic" directly into the event handlers, which
>> violates all manners of good OO design principles.
>
>
>> so you end up calling
>> self.gui.panel.textbox.GetValue() from within some other class which is
>> all sorts of wrong according to The Law of Demeter.
>
> Not if you follow this:
>
> http://wiki.wxpython.org/wxPython%20Style%20Guide
>
> But yes, you are right.
>
> It's a real pedagogical challenge - you want to be able to show folks how to use a given control without a whole lot of extra code, but then the natural thing for a newbie to do is use that code as a model, and, you're quite right, end up with spaghetti.
>
>
> I like mid-size, rather than tiny, examples, but it's really hard to show good code structure in a small program, no matter how you do it.
I don't think it often has to be that hard. Most simple cases, like sample code, could be refactored to provide more robust separation of concerns / MVC simply by deriving most of the sample classes from wx.EvtHandler instead of wx.WhateverView, then using self.control.Bind(event, self.myhandler.EventHandler) instead of self.Bind(event, self.EventHandler). e.g. here's something I did a few months back:
class STCFindReplaceController(wx.EvtHandler):
'''
This class controls Find and Replace behaviors for wxSTC, e.g. adding
"move to selection" behavior and setting up the keyboard shortcuts.
'''
def __init__(self, stc, *args, **kwargs):
wx.EvtHandler.__init__(self, *args, **kwargs)
self.stc = stc
self.searchText = None
self.lastFindResult = -1
self.BindEvents()
def OnKeyDown(self, event):
is_search = event.MetaDown() and event.KeyCode == 'G'
if self.searchText and is_search:
self.DoInlineSearch(self.searchText, next=True)
else:
event.Skip()
def DoInlineSearch(self, text, next=False, back=False):
self.searchText = text
startPos = 0
if self.lastFindResult > 0 and next:
startPos = self.lastFindResult + 1
self.lastFindResult = self.stc.FindText(0, self.stc.GetLength(), text)
if self.lastFindResult != -1:
self.stc.SetCurrentPos(self.lastFindResult)
self.stc.EnsureCaretVisible()
self.stc.SetSelectionStart(self.lastFindResult)
self.stc.SetSelectionEnd(self.lastFindResult + len(text))
def BindEvents(self):
self.stc.Bind(wx.EVT_KEY_DOWN, self.OnKeyDown)
Then in MyApp.OnInit or wherever appropriate:
self.frame = wx.Frame(None, -1, ...)
self.source = wx.stc.StyledTextCtrl(self.frame, -1)
self.findReplaceController = STCFindReplaceController(self.source)
So without hardly any extra code or added complexity at all, I've added my own behaviors to a wx.STC control without having to subclass wx.stc.StyledTextCtrl in my app. This way, if I create a new STC control somewhere else in my app, and want it also to have inline search capabilities (but not every other feature the other STC control has), then I don't need to do things like carefully plan out my subclass hierarchy into a multilayered jumble of MyFullFeaturedSTC, derived from MySTCWithFindReplace, derived from wx.stc.STC or use mixins to do much the same thing with multiple inheritance.... I can even dynamically enable / disable the functionality at runtime with a line or two of code. And, if I make the above code Windows-friendly in terms of keyboard shortcuts :), I could submit it to wx as a helper class that would save someone else the trouble of writing the same thing. One or two convenience classes like this won't make a big deal, but I suspect that over time they will grow and users will be able to just plug in a lot of advanced behaviors.
Thanks,
Kevin
> -Chris
>
>
>
> --
> Christopher Barker, Ph.D.
> Oceanographer
>
> Emergency Response Division
> NOAA/NOS/OR&R (206) 526-6959 voice
> 7600 Sand Point Way NE (206) 526-6329 fax
> Seattle, WA 98115 (206) 526-6317 main reception
>
> Chris....@noaa.gov
>
Raoul
This extraordinarily sticky subject goes 'way back to the early real-
time hardware days in which hardware events (AKA hardware-generated
"interrupt requests", "IRQ"s) needed to be "serviced" (AKA "handled"
by "IRQ handler" code routines) in the OS or kernel code. PC's and the
OS'es are not properly "real time" in that they can't guarantee that
an interrupt request/IRQ (analogous to a Python "event") can be
completed fast enough before other coprocesses (AKA, the MainLoop() in
Python) need those results or malfunction because they didn't get them
"soon enough", whatever the measure of that happens to be.
The manner in which this is resolved in all cases is that the time it
takes to "service the IRQ (or a Python "event") has to be evaluated to
determine whether or not it is likely or even guaranteed to be
serviced (completed) in a "short enough" time as not to impair the
running of the main program, which is the equivalent of the Python
MainLoop(). If it turns out that a too lengthy handler routine does
impair the overall program operation, then that handler routine (the
Python event handler) must set a flag or otherwise send a timely
message back to the main routine (or OS or kernel) that the
originating IRQ/event needs further processing for its completion and
that the main program must somehow schedule that to be done as another
"task" among all the other tasks that it already has to do.
If, using a real life example, the event simply takes too long to
complete, then the nuclear reactor in this particular example goes
critical and gets the "China Syndrome". In a pure software environment
the overall program operation will "stutter", produce bad data, or
simply crash or lock up.
The point of all this is:
It doesn't directly matter how many lines of code or the
complexity of that code the handler has to perform, but only the
amount of real-life "wall clock" time it takes to do it under *all*
circumstances. If it takes too long then the handler has to inform the
main program that it is, indeed, taking too long and that the main
program must wait in order not to proceed to do something that would
jeopardize the operation of the system.
Ray Pasco
Steven Sproat
I don't think you quite understood what I meant. I was not talking about
performance or events taking too long to complete, I was talking about
the practice of writing code that does all the "work" of your program
directly inside event handlers. I was also talking to some degree about
code that manipulates the properties and properties' properties of other
classes, as opposed to calling methods.
e.g. self.gui.control.textctrl.GetValue() could be written as a method
GetTextValue() on the control class, so we access
self.gui.control.GetTextValue(), or maybe even shifting the method into
the gui; self.giu.GetTextValue()
Steven Sproat
This is a really great post, Kevin. Thanks for that
Raoul
code related to completely handling the event right in the handler no
matter how complex it is.
The thing that has to be analyzed is if that processing takes too
long. Then, the handler has to tell the main program not to go along
and do something in the future that depends on that event being fully
processed when the handler is not finished. Otherwise the whole
program may get permanently "messed up".
The other thing to watch out for is if the the event processing just
takes too long. If your program controls the safety of something
critical like a nuclear reactor, or the fire control on a F-16, or
even the terminals for airline booking, then refactoring the the
program to make it react faster is in order. Or, perhaps, simply
substituting a faster computer to run the program.
The hardware/software analogy is this: software event processing has
to be treated with exactly the same strategies as hardware IRQ
processing when processing time is a critical factor. Otherwise the
system will fail, perhaps in a catastrophic manner.
Ray Pasco