Packaging her up

I've decided to try and package my classes into more appropriate locations instead of containing everything into one directory. I've just committed a repackaging of my GUI classes. I'd like to go further and split each file into smaller modules, with one classes each, especially for my "Tools".

However doing this will definitely break older save files and I'll have to monkey patch around it. I think I should move away from using Pickle as my save format, and perhaps use JSON instead. I just need to find a way to make a save converter for existing files...

Refactoring, refactoring

I've spent some time refactoring the program's code lately. The GUI class is getting really bloated with features and functionality that should be handled by a separate class. Thus, I've created a Menu class to create, bind and query the menu. Pretty good...

Also in the utility class, my save method was growing to over 100 lines, with many temporary variables being used. I managed to extract this into its own class, and split up the method into several smaller ones, passing parameters between them as needed. My temporary variables from the giant class became instance variables in the extracted class.

The end result is more code, but ultimately an easier to read, more abstract higher level save method - you just read the method calls it makes, as opposed to having to iterate over objects, save values etc inside the method.

I need to apply this to many more places in the program. I want to refactor out tab control to its own class, so that it can be shared by the Notes, thumbs and gui classes; these all share common data but it's duplicated per-class. Also the code is really hard to test - refactoring it out will definitely help.

Understanding Python decorators

I've finally come to grasp with Python's decorators, and have managed to create somewhat nicer code. Here's what I had before:

    def on_top(self, event):
        index, item = self.find_shape()
        self.shapes.pop(index)
        self.shapes.append(item)
        self.populate()
        self.list.Select(0)


    def on_bottom(self, event):
        index, item = self.find_shape()
        self.shapes.pop(index)
        self.shapes.insert(0, item)
        self.populate()
        self.list.Select(len(self.shapes) - 1)


    def on_up(self, event):
        index, item = self.find_shape()
        self.shapes.pop(index)
        self.shapes.insert(index + 1, item)
        x = self.list.GetFirstSelected() - 1
        self.populate()
        self.list.Select(x)


    def on_down(self, event):
        index, item = self.find_shape()
        self.shapes.pop(index)
        self.shapes.insert(index - 1, item)
        x = self.list.GetFirstSelected() + 1
        self.populate()
        self.list.Select(x)

As you can see - a lot of repeated code, with only different things happening in the middle of each function. This is ideal for a decorator. I struggled for a while to get it working as a decorator function inside a class, before reading how you define a wrapper function for creating a closure over the decorated function.

My code now reads,

    def move_shape(fn):
        """
        Passes the selected shape and its index to the decorated function, which
        handles moving the shape. function returns the list's index to select
        """
        def wrapper(self, event, index=None, item=None):
            index, item = self.find_shape()
            self.shapes.pop(index)
            x = fn(self, event, index, item)

            self.populate()
            self.list.Select(x)
        return wrapper

    @move_shape
    def on_top(self, event, index=None, item=None):
        self.shapes.append(item)
        return 0

    @move_shape
    def on_bottom(self, event, index=None, item=None):
        self.shapes.insert(0, item)
        return len(self.shapes) - 1

    @move_shape
    def on_up(self, event, index=None, item=None):
        self.shapes.insert(index + 1, item)
        return self.list.GetFirstSelected() - 1

    @move_shape
    def on_down(self, event, index=None, item=None):
        self.shapes.insert(index - 1, item)
        return self.list.GetFirstSelected() + 1

Unfortunately the event parameter is there because these are also wxPython event listeners, and I'd have to create delegate functions just to remove the one parameter, which seems pointless.