4
\$\begingroup\$

I'm still learning Python, so I tried to build a basic MVC app using wxPython and pubsub. While it seems to work fine, the code seems to be getting out of hand.

In model I decided against using traditional (e.g. Java) accessors and went with the more Pythonic approach of properties. I only wanted to set the setters, but ended up needing to set the getters too (because the variable name had to change to prevent recursion). What's the preferred way to do this?

Next, the flow for setting each variable seems a mess. Right now, changing the name in view publishes a message which controller is subscribed to, so it sets the name in model, which publisher another message which controller is subscribed to, which sends a message to view, which finally does something with the values (the last part is not really required at the moment, but should be in many cases). Is this all peachy, or am I missing something?

As for any other blunders, I'd be happy to hear them!

I mostly followed Mike Rooney's Code Sample.

"""
A simple attempt at a pubsub-driven MVC application with wxPython
"""
import wx
import wx.lib.pubsub

# this seems to be required -- bug?
pub = wx.lib.pubsub.Publisher()


class Model(object):
    def __init__(self, name, age):
        self._name = name
        self._age = age

    @property
    def name(self):
        return self._name

    @name.setter
    def name(self, value):
        self._name = value
        pub.sendMessage("name.changed", self.name)

    @property
    def age(self):
        return self._age

    @age.setter
    def age(self, value):
        self._age = value
        pub.sendMessage("age.changed", self.age)


class View(wx.Frame):
    def __init__(self, parent):
        wx.Frame.__init__(self, parent, title="MVC test", size=(200, 150))

        # setup sizers
        self.padding_sizer = wx.BoxSizer(wx.HORIZONTAL)
        self.sizer = wx.FlexGridSizer(2, 2, 8, 25)
        self.sizer.AddGrowableCol(1, 1) # make the second column fit available space
        self.padding_sizer.Add(self.sizer, proportion=1, flag=wx.ALL|wx.EXPAND, border=14)
        self.SetSizer(self.padding_sizer)

        # setup widgets

        # name
        self.name_label = wx.StaticText(self, label="Name:")
        self.name = wx.TextCtrl(self, style=wx.TE_PROCESS_ENTER)
        self.sizer.Add(self.name_label)
        self.sizer.Add(self.name, 1, wx.EXPAND)

        # age
        self.age_label = wx.StaticText(self, label="Age:")
        self.age = wx.TextCtrl(self, style=wx.TE_PROCESS_ENTER)
        self.sizer.Add(self.age_label)
        self.sizer.Add(self.age, 1, wx.EXPAND)

    # setters for wx controls
    def set_name(self, name):
        self.name.SetValue(name)

    def set_age(self, age):
        self.age.SetValue(str(age))


class Controller(object):
    def __init__(self, app):
        # controller holds refs to models, app and views
        self.model = Model('Goku', 9001)
        self.app = app
        self.view = View(None)

        # set up the view
        self.view.set_name(self.model.name)
        self.view.set_age(self.model.age)

        # bind view events to methods
        self.view.name.Bind(wx.EVT_TEXT_ENTER, self.change_name)
        self.view.age.Bind(wx.EVT_TEXT_ENTER, self.change_age)

        # subscriptions
        pub.subscribe(self.name_changed, "name.changed")
        pub.subscribe(self.age_changed, "age.changed")

        # finally,show the view
        self.view.Show()

    def change_name(self, evt):
        self.model.name = evt.String
        print "Name was changed to", self.model.name

    def name_changed(self, message):
        # unnecessary at the moment, for illustration only
        self.view.set_name(message.data)

    def change_age(self, evt):
        self.model.age = int(evt.String)
        print "Age was changed to", self.model.age

    def age_changed(self, message):
        # unnecessary at the moment, for illustration only
        self.view.set_age(message.data)


if __name__ == '__main__':
    #create the wx app
    app = wx.App(False)
    # pass the app to the controller
    controller = Controller(app)
    # start the app running
    app.MainLoop()

Screenshot

\$\endgroup\$
4
\$\begingroup\$

It mostly looks pretty good. I have just a couple tweaks to suggest.

You have

import wx.lib.pubsub

# this seems to be required -- bug?
pub = wx.lib.pubsub.Publisher()

The example from the wxpython wiki does

from wx.lib.pubsub import Publisher as pub

I think using that form of the import will let you drop the line where you instantiate the Publisher.

I like to put my mainline code in a main() routine right after the imports so it's clear where the work begins

def main(args):
    #create the wx app
    app = wx.App(False)

    # pass the app to the controller
    controller = Controller(app)

    # start the app running
    app.MainLoop()

Then at the bottom of the file,

if __name__ == '__main__':
    # call main rather than hiding the payload down here
    main(sys.argv)
\$\endgroup\$
2
\$\begingroup\$

The question is if you need your model to be able to notify other subscribers (controllers) about a change of the name or not.

If your controller is the only class interacting with the model (as it seems, since the Model is created in the Controller's __init__), you could get rid of using pubsub in the Model and halve the number of methods in the Controller.

For example, reduced to the methods related to changing the name, you could write:

class Model(object):
def __init__(self, name, age):
    self._name = name
    self._age = age

@property
def name(self):
    return self._name

@name.setter
def name(self, value):
    self._name = value
    # --> pubsub call removed here

# ...

class Controller(object):

# left remaining methods out

def change_name(self, evt):
    self.model.name = evt.String
    print "Name was changed to", self.model.name
    self.view.set_name(message.data)

# method name_changed not needed any longer

Another point worth mentioning is that with your current implementation your Controller class needs to know about the implementation of the View class (i.e. that it is using wxPython and what kind of events it has to subscribe to). You should leave the binding of the events to the view class. Instead of utilizing pubsub in the Model, it would be of much more use in the View class.

Define a callback for the relevant UI events inside your View class and bind it to wx.EVT_TEXT_ENTER. When a new name is entered, the View notifies the Controller with pubsub. The Controller then takes care of your logic like validating the input, updating the Model and if necessary trigger additional actions on the View or Model or dealing with errors like asking the View to display an error message if the name is invalid.

Again using the Name field as example:

from wx.lib.pubsub import Publisher as pub

class Model(object):
    def __init__(self, name, age):
        self._name = name
        self._age = age

    @property
    def name(self):
        return self._name

    @name.setter
    def name(self, value):
        self._name = value


class View(wx.Frame):
    def __init__(self, parent):
        wx.Frame.__init__(self, parent, title="MVC test", size=(200, 150))

        # setup sizers
        self.padding_sizer = wx.BoxSizer(wx.HORIZONTAL)
        self.sizer = wx.FlexGridSizer(2, 2, 8, 25)
        self.sizer.AddGrowableCol(1, 1) # make the second column fit available space
        self.padding_sizer.Add(self.sizer, proportion=1, flag=wx.ALL|wx.EXPAND,         border=14)
        self.SetSizer(self.padding_sizer)

        # setup widgets

        # name
        self.name_label = wx.StaticText(self, label="Name:")
        self.name = wx.TextCtrl(self, style=wx.TE_PROCESS_ENTER)
        self.sizer.Add(self.name_label)
        self.sizer.Add(self.name, 1, wx.EXPAND)
        # --> NEW: bind event in View class
        self.name.Bind(wx.EVT_TEXT_ENTER, self.on_name_changed)

    # setters for wx controls
    def set_name(self, name):
        self.name.SetValue(name)

    # callback for UI events
    def on_name_changed(self, event):
        new_name = event.String
        pub.sendMessage("name.changed", new_name)


class Controller(object):
    def __init__(self, app):
        # controller holds refs to models, app and views
        self.model = Model('Goku', 9001)
        self.app = app
        self.view = View(None)

        # set up the view
        self.view.set_name(self.model.name)
        self.view.set_age(self.model.age)

        # DELETED: Binding of UI events removed

        # subscriptions
        pub.subscribe(self.name_changed, "name.changed")

        # finally,show the view
        self.view.Show()

    def name_changed(self, message):
        self.model.name = evt.String
        print "Name was changed to", self.model.name
        # unnecessary at the moment, for illustration only
        self.view.set_name(message.data)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.