One nice thing about gentle refactoring is that often we get ideas for how to make things better. Today …

Let’s see what we can do to smooth things out. We’ll start with Wheel, which is rather a mess:

class Wheel:
    def __init__(self, *, parent, angle_degrees=0):
        self.line = None
        self.circle = None
        self.parent = parent
        self._angle_radians = angle_degrees * math.pi / 180.0

    def update(self, start, finish):
        self.angle_degrees = self.angle_degrees + 4
        return self.start, self.finish

    @property
    def parent(self):
        return self._position
    @parent.setter
    def parent(self, value):
        self._position = value

    @property
    def start(self):
        return self.parent
    @property
    def finish(self):
        return self.crank_position(1, self._angle_radians)

    def crank_offset(self, radius, tilt_radians):
        effective_angle = self._angle_radians + tilt_radians
        return vector(radius, 0).rotate_xy(effective_angle)

    def crank_position(self, radius, tilt_radians):
        return self.crank_offset(radius, tilt_radians) + self.parent

    @property
    def angle_radians(self):
        return self._angle_radians
    @angle_radians.setter
    def angle_radians(self, radians):
        self._angle_radians = radians

    @property
    def angle_degrees(self):
        return self._angle_radians*180.0/math.pi
    @angle_degrees.setter
    def angle_degrees(self, degrees):
        self._angle_radians = degrees * math.pi / 180.0

That’s abominable, and worse, most of those properties are actually referred to from elsewhere. I’ll begin by checking how all these things are being used. The Wheel should be simple, with its center as start and a point at radius 1, at the angle the wheel is currently rotated, as finish. You’d think that would be all.

I find a test that can be recast to use finish:

    def test_wheel_finish(self):
        loc = vector(2, 3)
        wheel = Wheel(parent=loc, angle_degrees=45)
        crank_offset = wheel.finish - loc
        sqrt2by2 = math.sqrt(2)/2
        assert crank_offset.x == pytest.approx(sqrt2by2, 0.0001)
        assert crank_offset.y == pytest.approx(sqrt2by2, 0.0001)
        crank_position = wheel.finish
        assert crank_position.x == pytest.approx(2+sqrt2by2, 0.0001)
        assert crank_position.y == pytest.approx(3+sqrt2by2, 0.0001)

Now I can inline some of what’s in Wheel. After some judicious inlining, and a little less judicious, Wheel is down to this, net of its drawing code:

class Wheel:
    def __init__(self, *, parent, angle_degrees=0):
        self.line = None
        self.circle = None
        self._position = parent
        self._angle_radians = angle_degrees * math.pi / 180.0

    def update(self, start, finish):
        self._angle_radians = self._angle_radians + 4*math.pi / 180.0
        return self.start, self.finish

    @property
    def start(self):
        return self._position
    @property
    def finish(self):
        return self._position + vector(1, 0).rotate_xy(self._angle_radians)

Tests are green, the animation is still drawn correctly. Commit: refactor Wheel, removing mass quantities of properties.

Now what? I think Crank is referring now to lots of Wheel’s private members, asking for angles and such. Let’s have a look:

class Crank:
    def __init__(self, *, parent, radius, lead_degrees=0):
        self._parent = parent
        self._radius = radius
        self._leadRadians = lead_degrees * math.pi / 180.0

    def update(self, start, finish):
        pass

    @property
    def parent(self):
        return self._parent
    @property
    def start(self):
        return self.parent.start
    @property
    def finish(self):
        return self.parent.finish

    def start_finish(self):
        start = self._parent.start
        finish = self.constrained(0)
        return start, finish

    def constrained(self, number):
        center = self._parent.start
        r=self._radius
        effective_angle = self._leadRadians + self._parent._angle_radians
        x = r*math.cos(effective_angle) - 0*math.sin(effective_angle)
        y = r*math.sin(effective_angle) + 0*math.cos(effective_angle)
        return vector(x, y) + center

    def origin(self):
        return self._parent.start

I don’t think finish can possibly be right. See start_finish just below. Change all that:

    @property
    def parent(self):
        return self._parent
    @property
    def start(self):
        return self._parent.start
    @property
    def finish(self):
        return self.constrained(0)

    def start_finish(self):
        return self.start, self.finish

But the real issue is that update isn’t doing anything at all, and of course we’re referring to parent all over the place.

I update update like this:

    def update(self, p_start, p_finish):
        dif = p_finish - p_start
        parent_radians = math.atan2(dif.y, dif.x)
        assert parent_radians == self._parent._angle_radians
        return self.start, self.finish

The tests run. So I inserted an assert False into the method and it still runs. I am confused.

After a bit of study, I am no longer confused, only gobsmacked. I had never added Crank to the drawing collection. Furthermore, the Crank does not know how to draw itself. I think I can bypass that for the moment: it would only draw a little circle or something. Yes. The draw draws OK now. Tests run. Commit a save point.

I have to go to a meeting now, let’s sum up.

Summary

A bit more progress. Very nice cleanup in Wheel, cut it in half or better. Crank is coming along and despite that it draws nothing, it’s doing the right calculations. My assertion (modified to allow for rounding) is passing in the update, a step on the way to computing the info we need locally in Crank.

Decent steps. More next time. And a report on FGNO.

See you then!