RFC: tkSimpleDialog IMPROVED AGAIN!.

rantingrick rantingrick at gmail.com
Sun Jul 3 17:09:09 EDT 2011


Hello Folks,

As many of you already know yours truly has posted improvements to the
tkSimpleDialog many moons back HOWEVER i was looking over the code a
few moments ago and realized even more improvements were needed.

These improvements mainly concern naming conventions, comments, and
docstrings although i have some deeper and profound thoughts on
virtual functions/methods that i will probably save for another thread
in the very near future.

As for tkSimpleDialog.py I had to change quite a bit of python style
guide abominations in this and previous edits.

-----------------------
First let's see the code in it's ORIGINAL STATE: (Warning: freak show
ahead!)
-----------------------

## Start Code: tkSimpleDialog.Dialog ##
from Tkinter import *

class Dialog(Toplevel):

    '''Class to open dialogs.

    This class is intended as a base class for custom dialogs
    '''

    def __init__(self, parent, title = None):

        '''Initialize a dialog.

        Arguments:

            parent -- a parent window (the application window)

            title -- the dialog title
        '''
        Toplevel.__init__(self, parent)

        # If the master is not viewable, don't
        # make the child transient, or else it
        # would be opened withdrawn
        if parent.winfo_viewable():
            self.transient(parent)

        if title:
            self.title(title)

        self.parent = parent

        self.result = None

        body = Frame(self)
        self.initial_focus = self.body(body)
        body.pack(padx=5, pady=5)

        self.buttonbox()

        self.wait_visibility() # window needs to be visible for the
grab
        self.grab_set()

        if not self.initial_focus:
            self.initial_focus = self

        self.protocol("WM_DELETE_WINDOW", self.cancel)

        if self.parent is not None:
            self.geometry("+%d+%d" % (parent.winfo_rootx()+50,
                                      parent.winfo_rooty()+50))

        self.initial_focus.focus_set()

        self.wait_window(self)

    def destroy(self):
        '''Destroy the window'''
        self.initial_focus = None
        Toplevel.destroy(self)

    #
    # construction hooks

    def body(self, master):
        '''create dialog body.

        return widget that should have initial focus.
        This method should be overridden, and is called
        by the __init__ method.
        '''
        pass

    def buttonbox(self):
        '''add standard button box.

        override if you do not want the standard buttons
        '''

        box = Frame(self)

        w = Button(box, text="OK", width=10, command=self.ok,
default=ACTIVE)
        w.pack(side=LEFT, padx=5, pady=5)
        w = Button(box, text="Cancel", width=10, command=self.cancel)
        w.pack(side=LEFT, padx=5, pady=5)

        self.bind("<Return>", self.ok)
        self.bind("<Escape>", self.cancel)

        box.pack()

    #
    # standard button semantics

    def ok(self, event=None):

        if not self.validate():
            self.initial_focus.focus_set() # put focus back
            return

        self.withdraw()
        self.update_idletasks()

        try:
            self.apply()
        finally:
            self.cancel()

    def cancel(self, event=None):

        # put focus back to the parent window
        if self.parent is not None:
            self.parent.focus_set()
        self.destroy()

    #
    # command hooks

    def validate(self):
        '''validate the data

        This method is called automatically to validate the data
before the
        dialog is destroyed. By default, it always validates OK.
        '''

        return 1 # override

    def apply(self):
        '''process the data

        This method is called automatically to process the data,
*after*
        the dialog is destroyed. By default, it does nothing.
        '''

        pass # override
## End Code: tkSimpleDialog.Dialog##

As you can see this code reeks of abominations. Not only is the
interface poorly designed, but we have poor naming conventions, too
much vertical spacing, unhelpful and poorly formatted doc-strings, not
enough comments where needed and too many in other places. Not the
kind of code you bring home for mom to stick on the fridge. D:

-----------------------
Now lets see the code in it's CURRENT STATE (with cleaned up comments
and doc-strings!)
-----------------------

## Start Code: New and improved dialog! ##
import Tkinter as tk

class Dialog(tk.Toplevel):
    def __init__(self, parent, title='Dialog'):
        """
        Initialize a dialog;
            parent:
                A Tkinter.Toplevel instance.
            title:
                The dialog's title as a string.
        """
        tk.Toplevel.__init__(self, parent)
        self.withdraw()
        if parent.winfo_viewable():
            # If the parent window is not viewable, don't
            # make the child transient, or else it would be
            # opened withdrawn!
            self.transient(parent)
        if title:
            self.title(title)
        self.parent = parent
        # self.result is where we store any values for return to
        # the caller.
        self.result = None
        # We save the dialog's main widget frame just incase a
        # caller needs to access it later.
        self.frame = tk.Frame(self)
        # Next we call self.body to create the body of the dialog.
        # We save the return value as "self.initial_focus" so we
        # give it focus later.
        self.initial_focus = self.body(self.frame)
        self.frame.pack(padx=5, pady=5)
        self.buttonbox()

    def show(self):
        # XXX: Need "show" and "show_modal" methods!
        self.deiconify()
        # The window needs to be visible for the grab so we
        # call wait_visibility and let Tkinter do the waiting
        # for us.
        self.wait_visibility()
        self.grab_set()
        if not self.initial_focus:
            self.initial_focus = self
        self.protocol("WM_DELETE_WINDOW", self.cancel)
        if self.parent is not None:
            self.geometry(
                "+%d+%d" %(
                    self.parent.winfo_rootx()+50,
                    self.parent.winfo_rooty()+50)
                )
        self.initial_focus.focus_set()
        self.wait_window(self)
        return self.result

    def destroy(self):
        '''Destroy the window'''
        self.initial_focus = None
        tk.Toplevel.destroy(self)
    #
    # Construction Hooks.
    #

    def body(self, master):
        # XXX: create_body
        '''
        OVERIDE;
        Create dialog body and return widget that should have
        initial focus.
        '''
        pass

    def buttonbox(self):
        # XXX: create_buttonbox
        # XXX: Overiding this method can break funcionality if you do
not
        #      properly bind the ok and cancel buttons correctly!
        # RFC: Should we create a generic button creation methos or a
mixin?
        '''add a standard button box;
        override if you do not want differnt buttons.
        '''
        box = tk.Frame(self)
        w = tk.Button(box, text="OK", width=10, command=self.ok,
                      default=tk.ACTIVE)
        w.pack(side=tk.LEFT, padx=5, pady=5)
        w = Button(box, text="Cancel", width=10, command=self.cancel)
        w.pack(side=tk.LEFT, padx=5, pady=5)
        self.bind("<Return>", self.ok)
        self.bind("<Escape>", self.cancel)
        box.pack()

    #
    # Standard Button Callbacks.
    #

    def ok(self, event=None):
        # XXX: okButtonCallback | callbackOkButton | (cb*|*cb)
        """ Proces the data automatically when user presses ok
button."""
        if not self.validate():
            self.initial_focus.focus_set() # put focus back
            return
        self.withdraw()
        self.update_idletasks()
        try:
            self.apply()
        finally:
            self.cancel()

    def cancel(self, event=None):
        # XXX: cancelButtonCallback | callbackCancelButton | (cb*|*cb)
        """ Return focus to the parent window and destroy self"""
        if self.parent is not None:
            self.parent.focus_set()
        self.destroy()

    #
    # Command Hooks.
    #

    def validate(self):
        # XXX: data_validate
        """
        Validate the data;
        This method is called automatically to validate the data
before the
        dialog is destroyed. By default, it always validates True.

        Overide to change behavoir to something more suitabe for your
        derived dialog.
        """
        return True

    def apply(self):
        # XXX: data_process
        """
        Process the data;
        This method is called automatically to process the data but
only
        *after* the dialog is destroyed. By default, it does nothing.

        Overide to change the behavoir to something more suitabe for
your
        derived dialog
        """
        pass

if __name__ == "__main__":
    root = Tk()
    import tkMessageBox
    ERROR_STR = "We don't have any \"{0}\"!"

    class MyDialog(Dialog):

        def body(self, master):
            self.strvar = tk.StringVar(master, "Eggs")
            self.label = tk.Label(self, text='Default')
            self.label.pack()
            self.options = OptionMenu(
                self,
                self.strvar,
                "Jam",
                "Ham",
                "Spam",
                )
            self.options.pack()

        def validate(self):
            text = self.strvar.get()
            if text and text.lower() != 'spam':
                tkMessageBox.showerror(
                    'Order Error',
                    ERROR_STR.format(text),
                    parent=self
                    )
                return False
            return True

        def apply(self):
            self.result = self.strvar.get()

    m = MyDialog(root, title='Menu = spam')
    # Test changing widget values after initialization.
    m.label['text'] = 'What would you like to order?'
    # Show the modal dialog.
    result = m.show()
    print 'User Choose: {0}'.format(result)

    root.mainloop()
## End Code: New and improved dialog!  ##

Ahhh. As you can see i removed the cruft, cleaned up the spacing,
formatted, improved the doc stings and comments AND even created a
nice little test case at the bottom. Folks this is how a pythonista
writes code. This is what you should stive for every day!  HOWEVER  i
believe we can do even better than this Oh Yes!!!

-----------------------
 Name changes
-----------------------
body -> create_body
buttonbox -> create_buttonbox
validate -> data_validate | validate_data
apply -> data_process | process_data

-----------------------
 Methods
-----------------------
Should we have show_modal and show methods? I believe so!

-----------------------
 Pitfalls
-----------------------
Overiding the buttonbox method can be dangerous since bindings must be
set correctly or the cancel and apply methods will break! I think we
can improve this code by either (A) a mixin buttonbox class OR
abstracting the button box creation and passing in a type string to
the SimpleDialog intial method...


RFC... bring it on folks!




More information about the Python-list mailing list