Today we do a very nice step by step refactoring to a Method Object. Quite pleasing, really. Yummy.

As reported yesterday, the methods of ConRod are:

def __init__
def crank_radians
def crank_radius
def draw
def init_draw
def init_guide_line
def init_minimum_travel_point
def init_rod_background
def init_rod_picture
def update

Of these, crank_radians and crank_radius are static methods used in update. The update method, of course is really the only operational method of our component objects, the method that returns their coordinates for use in further updating.

For testing convenience, our components also have a method draw, which draws a representation of that component on a provided Canvas object. And because our components typically have one or more long-lived objects representing the drawn component, we have a method init_draw that sets up what the objects need to remember.

All those other init methods are part of initializing the ConRod drawing, as extracted yesterday in the name of better expressing how the init_draw works.

I do prefer the more expressive breakout of methods for init_draw. I do not like six of the ten methods of ConRod being about drawing, which isn’t even its primary function. Our mission today is to figure out what, if anything, we might choose to do about this concern.

Possibilities are many, including:

  1. Ignore the problem: the code is readily understood and only ten methods;
  2. Rename the init methods to be private, so that code readers will be drawn to the more important methods;
  3. Separate the drawing methods from the updating methods lexically, by arranging the file to draw attention to the important things first.
  4. Eliminate the helpful background guide lines from the drawing, removing most of the init code;
  5. Create a new utility object to do the drawing, so that the ConRod only needs to create it and defer drawing to it.

It’s this last idea that we’ll explore today. In part, what we are doing is very similar to the “Method Object” refactoring, which replaces a complex method with an object. We’re not doing quite the same thing: we may need to support two methods with our object, init_draw and draw. We’ll see.

Our current component protocol requires implementation of init_draw and draw. Prediction is difficult, especially about the future, but I think our object might allow us to have just one call. For example, it might init on creation, or it might have a lazy init for the first time that draw is called. Probably the former: the latter is a bit fancy and should be avoided on that basis.

ConRod is probably not the simplest place to put our first attempt at whatever it is we’re about to do, because our drawing code wants to know the center of rotation of the crank that drives our rod, and the ConRod itself does not know or care about that value. A wise person, were one close to hand, might try this scheme on one of the simpler objects. But we are here now, and we’ll work from here.

Let’s add an empty object _painter to our class:

class ConRod:
    def __init__(self, *, length, piston_angle):
        self._length = length
        self._piston_radians = piston_angle * math.pi / 180
        # mutable graphic support temps
        self._painter = None
        self._cached_line = None
        self._cached_wheel_center = None

We’re inventing a new convention, doing our best to keep things general, without doing any extra work that isn’t called for.

Note
I’d really like to be doing some tests to drive this object. I’m not sure just how to do that but I think I’d be wise to create a test file just to serve as an attractor for tests when they come to mind.
class ConRodPainter:
    def __init__(self, painter):
        self._painter = painter


class TestConRodPainter:
    def test_exists(self):
        rod = ConRod(length=5,piston_angle=0)
        painter = ConRodPainter(rod)

Given the creation parameter, we can create the painter right in ConRod’s init:

class ConRod:
    def __init__(self, *, length, piston_angle):
        self._length = length
        self._piston_radians = piston_angle * math.pi / 180
        # mutable graphic support temps
        self._painter = ConRodPainter(self)
        self._cached_line = None
        self._cached_wheel_center = None

Now let’s do init_draw. That gets called on the ConRod and currently looks like this:

    def init_draw(self, canvas, start, finish):
        self.init_rod_picture(canvas, start, finish)
        self.init_rod_background(canvas)

We’re going to defer that to our new object. Let’s review the full init: it references a couple of members now:

    def init_draw(self, canvas, start, finish):
        self.init_rod_picture(canvas, start, finish)
        self.init_rod_background(canvas)

    def init_rod_picture(self, canvas, start, finish):
        self._cached_line = canvas.create_line(
            start.x, start.y,
            finish.x, finish.y,
            width=3, fill="red")

    def init_rod_background(self, canvas):
        wheel_center = self._cached_wheel_center
        point_on_rod_line = vector(2 * self._length, 0, 0).rotate_xy(self._piston_radians)
        self.init_guide_line(canvas, wheel_center, point_on_rod_line)
        self.init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

    def init_guide_line(self, canvas, wheel_center, point_on_rod_line):
        canvas.create_line(wheel_center.x, wheel_center.y, wheel_center.x + point_on_rod_line.x,
                           wheel_center.y + point_on_rod_line.y, fill="blue")

    def init_minimum_travel_point(self, canvas, wheel_center, point_on_rod_line):
        one = vector(1, 0).rotate_xy(self._piston_radians)
        half = wheel_center + one + point_on_rod_line * 0.5
        s = 0.1
        canvas.create_oval(half.x - s, half.y - s, half.x + s, half.y + s, fill='red')

There are three references to self, the ConRod. We access _length, _cached_wheel_center, and _piston_radians. Our existing code can compute _length from the start and finish. We’d like to minimize the connections between the ConRod and its Painter, so let’s change that in the existing code:

    def init_draw(self, canvas, start, finish):
        length = finish.distance(start)
        self.init_rod_picture(canvas, start, finish)
        self.init_rod_background(canvas, length)

    def init_rod_background(self, canvas, length):
        wheel_center = self._cached_wheel_center
        point_on_rod_line = vector(2 * length, 0, 0).rotate_xy(self._piston_radians)
        self.init_guide_line(canvas, wheel_center, point_on_rod_line)
        self.init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

That works and draws correctly. We can commit, so let’s do: compute rod length in drawing code.

Now we have an issue. As things stand now, the ConRod angle is fixed and defined at creation time. I can imagine a conrod that doesn’t have a fixed basic angle, but that’s not this object. So we can and should pass that angle to the Painter. Change the test:

class ConRod:
    def __init__(self, *, length, piston_angle):
        self._length = length
        self._piston_radians = piston_angle * math.pi / 180
        # mutable graphic support temps
        self._painter = ConRodPainter(self, self._piston_radians)
        self._cached_line = None
        self._cached_wheel_center = None

I forgot to do that with Change Signature, actually typed in the letters like some kind of barbarian. No harm done. Commit again: added piston radians parm to painter.

Now it is time to start moving function from ConRod over to our ConRodPainter. We begin with a trick:

class ConRod:
    def init_draw(self, canvas, start, finish):
        self._painter.init_draw(canvas, start, finish)

class ConRodPainter:
    def init_draw(self, canvas, start, finish):
        length = finish.distance(start)
        self._rod.init_rod_picture(canvas, start, finish)
        self._rod.init_rod_background(canvas, length)

I just moved the one method over and then call back to ConRod to do the work. Everything is still wired up OK. We can and do commit: moving drawing to painter.

Move and adjust one method:

    def init_draw(self, canvas, start, finish):
        length = finish.distance(start)
        self.init_rod_picture(canvas, start, finish)
        self._rod.init_rod_background(canvas, length)

    def init_rod_picture(self, canvas, start, finish):
        self._cached_line = canvas.create_line(
            start.x, start.y,
            finish.x, finish.y,
            width=3, fill="red")
        self._rod._cached_line = self._cached_line

I’m caching the line in both places for now, since the moving isn’t complete. Drawing good. Commit again. Move the next method. I copy it, paste it to the Painter, and edit:

class ConRodPainter:
    def init_rod_background(self, canvas, length):
        wheel_center = self._rod._cached_wheel_center
        point_on_rod_line = vector(2 * length, 0, 0).rotate_xy(self._piston_radians)
        self._rod.init_guide_line(canvas, wheel_center, point_on_rod_line)
        self._rod.init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

So far so good. The drawing continues to work. Commit again. Move method:

    def init_guide_line(self, canvas, wheel_center, point_on_rod_line):
        canvas.create_line(wheel_center.x, wheel_center.y, wheel_center.x + point_on_rod_line.x,
                           wheel_center.y + point_on_rod_line.y, fill="blue")

Drawing still works. Commit again. Move another method:

    def init_minimum_travel_point(self, canvas, wheel_center, point_on_rod_line):
        one = vector(1, 0).rotate_xy(self._piston_radians)
        half = wheel_center + one + point_on_rod_line * 0.5
        s = 0.1
        canvas.create_oval(half.x - s, half.y - s, half.x + s, half.y + s, fill='red')

Works. Commit. The only un forwarded method left in ConRod is draw. Move that.

class ConRod:
    def draw(self, canvas, start, finish):
        self._painter.draw(canvas, start, finish)

class ConRodPainter:
    def draw(self, canvas, start, finish):
        canvas.coords(self._cached_line,
                      start.x, start.y,
                      finish.x, finish.y)

Drawing works. Commit: drawing moved to ConRodPainter.

Let’s review the changes in ConRod and the entire ConRodPainter:

class ConRod:
    def init_draw(self, canvas, start, finish):
        self._painter.init_draw(canvas, start, finish)

    def draw(self, canvas, start, finish):
        self._painter.draw(canvas, start, finish)


class ConRodPainter:
    def __init__(self, rod, piston_radians):
        self._rod = rod
        self._piston_radians = piston_radians
        self._cached_line = None

    def init_draw(self, canvas, start, finish):
        length = finish.distance(start)
        self._init_rod_picture(canvas, start, finish)
        self._init_rod_background(canvas, length)

    def draw(self, canvas, start, finish):
        canvas.coords(self._cached_line,
                      start.x, start.y,
                      finish.x, finish.y)

    def _init_rod_picture(self, canvas, start, finish):
        self._cached_line = canvas.create_line(
            start.x, start.y,
            finish.x, finish.y,
            width=3, fill="red")
        self._rod._cached_line = self._cached_line

    def _init_rod_background(self, canvas, length):
        wheel_center = self._rod._cached_wheel_center
        point_on_rod_line = vector(2 * length, 0, 0).rotate_xy(self._piston_radians)
        self._init_guide_line(canvas, wheel_center, point_on_rod_line)
        self._init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

    def _init_guide_line(self, canvas, wheel_center, point_on_rod_line):
        canvas.create_line(wheel_center.x, wheel_center.y, wheel_center.x + point_on_rod_line.x,
                           wheel_center.y + point_on_rod_line.y, fill="blue")

    def _init_minimum_travel_point(self, canvas, wheel_center, point_on_rod_line):
        one = vector(1, 0).rotate_xy(self._piston_radians)
        half = wheel_center + one + point_on_rod_line * 0.5
        s = 0.1
        canvas.create_oval(half.x - s, half.y - s, half.x + s, half.y + s, fill='red')

I made all the obviously private methods private before I pasted. Commit: tidying.

Now we still have some accesses from the ConRodPainter to the ConRod. What are they? Here’s the first, no longer needed:

    def _init_rod_picture(self, canvas, start, finish):
        self._cached_line = canvas.create_line(
            start.x, start.y,
            finish.x, finish.y,
            width=3, fill="red")
        self._rod._cached_line = self._cached_line

Remove that. Drawing good. Commit: remove a back-reference. There is another:

    def _init_rod_background(self, canvas, length):
        wheel_center = self._rod._cached_wheel_center
        point_on_rod_line = vector(2 * length, 0, 0).rotate_xy(self._piston_radians)
        self._init_guide_line(canvas, wheel_center, point_on_rod_line)
        self._init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

We could pass that on init_draw. Let’s do that.

class ConRod:
    def init_draw(self, canvas, start, finish):
        self._painter.init_draw(canvas, start, finish, self._cached_wheel_center)

class ConRodPainter:
    def init_draw(self, canvas, start, finish, wheel_center):
        length = finish.distance(start)
        self._init_rod_picture(canvas, start, finish)
        self._init_rod_background(canvas, length, wheel_center)

    def _init_rod_background(self, canvas, length, wheel_center):
        point_on_rod_line = vector(2 * length, 0, 0).rotate_xy(self._piston_radians)
        self._init_guide_line(canvas, wheel_center, point_on_rod_line)
        self._init_minimum_travel_point(canvas, wheel_center, point_on_rod_line)

Things draw correctly. Are there any references back to the rod now? There are not. We remove it from the init and change the signature:

class ConRodPainter:
    def __init__(self, piston_radians):
        self._piston_radians = piston_radians
        self._cached_line = None

Commit: back references removed.

Voila!

We have moved all the drawing-related code over to a ConRodPainter object which is no more complex than the code was when it was inside ConRod, and which makes no references to ConRod. ConRod still caches the wheel center, and passes it to the init_draw, because we want to draw that nice little line and dot.

We did it in a series of twelve individual commits, with the drawing all working all the time. We have no useful tests for the ConRodPainter and none came to mind as we worked. I’ll try to reflect on that in my copious free time. Without some kind of dummy Canvas object, I’m not sure what we could have done that would have been useful. We’ll keep the file: perhaps it will attract tests later.

Most of the work consisted of cutting a method from ConRod, pasting it into ConRodPainter, and editing it a bit to allow for the callbacks. We kept calling back to ConRod until there was nothing left to call, moving the code over one method at a time.

Could we have done it all in one go? Perhaps. But why would we? It couldn’t have gone more smoothly, and might not have gone as smoothly. Might it have taken less time? I doubt it but possibly. Jumping across the creek is faster than walking on the crossing stones, unless you don’t quite make it, in which case you have to add in the time spent drying your clothes. I’d rather walk on the crossing stones.

Small steps are just so nice. We’ll reflect a bit on what we have here next time, to see whether we want to canonize this approach for all our components.

See you next time!