We reviewed the linkage program a bit at FGNO on Tuesday night. Some thoughts arose. My big lesson: Shut up and listen.

As usual, Tuesday night the Friday Geeks Night Out group met on Zoom. I had requested that we review the linkage code. I had three somewhat specific questions about the current code:

  1. Which form of the conrod calculation should be preferred, the compact one or the highly refactored one;
  2. What did the group think about the way updates are handled;
  3. What should be done about the draw capability being bound into the same object as the calculation;
  4. What should be done about the objects all being mutable?

One result of my having questions was that I probably missed out on observations that the group might have made had they been left on their own. Possibly I should have been more passive during the session: I might have learned something more than I did. If I have a flaw, it is somewhere near this kind of happening.

Compact or Refactored?

Let’s look at the compact vs refactored question first. Here are the two styles for ConRod. Note that the compact form now takes a slightly different calling sequence than the refactored version, but the concern here is the style more than the details.

Compact Form

def con_rod_ends_complete_inline(
        *, piston_radians, origin, radius,
        lead_radians, wheel_radians, length):
    crank_radians = wheel_radians + lead_radians
    effective_radians = crank_radians - piston_radians
    crank_offset = vector(radius, 0).rotate_xy(effective_radians)
    x, y = crank_offset.x, crank_offset.y
    x_projection = math.sqrt(length * length - y * y)
    constrained_end = vector(x + x_projection, 0)
    return (origin + crank_offset.rotate_xy(piston_radians),
            origin + constrained_end.rotate_xy(piston_radians))

Refactored Form

def con_rod_ends_complete(*, 
        piston_radians, 
        wheel_center, 
        radius, 
        crank_radians, 
        length):
    effective_radians = rotate_to_horizontal(crank_radians, piston_radians)
    start, finish = con_rod_ends(radius, effective_radians, length)
    sr, fr = rotate_inverse(start, finish, piston_radians)
    return translate_to_wheel_center(sr, fr, wheel_center)

def rotate_to_horizontal(crank_radians, piston_radians):
    return crank_radians - piston_radians

def rotate_inverse(start, finish, piston_radians):
    sr = start.rotate_xy(piston_radians)
    fr = finish.rotate_xy(piston_radians)
    return sr, fr

def translate_to_wheel_center(sr, fr, origin):
    return sr + origin, fr + origin

def con_rod_ends(crank_radius, crank_angle, length):
    # zero-based, horizontal
    crank_offset = vector(crank_radius,0).rotate_xy(crank_angle)
    constrained_end = con_rod_solve(crank_offset, length)
    return crank_offset, constrained_end

def con_rod_solve(crank_offset, length):
    x,y = crank_offset.x, crank_offset.y
    x_projection = math.sqrt(length*length - y*y)
    return vector(x+x_projection,0)

It turns out that our group uniformly prefers the refactored form because it is more expressive, so that when it becomes necessary to understand how it works, the clues are more explicit: here we rotate, here we rotate back, and so on. None of us is concerned about the presumed performance impact of the additional function calls required by this code.

A couple of comments are worth noting:

  1. If the whole team was all about the geometry and how it works, the compact form might be OK because everyone could understand it. I think most of us would use the refactored form even then.

  2. There is some possibility of reuse of the smaller functions. We mostly consider that to be a very small additional bit of weight on the refactored side, although in some quarters the argument for reuse might weigh more heavily. My personal style is to discover reuse, not to prepare for it.

  3. The name ConRod presumably stands for ConnectingRod, and one might wonder why we didn’t use the full name. I believe I pretended not to hear that perfectly legitimate observation. If all the railroad people called it a “con rod”, then the name would be appropriate. If they call it “connecting rod”, then we should call it that in the code. I believe, but am not certain, that it is called “main rod” by railroad people, but as an auto person I think “con rod”. Naming is important, clarity is important, consistency is important. I embody none of those on my worst days, some on my best.

Asymmetry

As we explored the compact code, GeePawHill observed that although I was thinking in terms of translate, rotate, operate, rotate inverse, translate inverse, there was no actual translate happening, and the call that used to be there to transform_inverse was therefore confusing him and causing a stack overflow in his brain.

I realized that I was too much in love with that model, so we revised the compact version a bit so that it doesn’t lean on the translate/inverse but does retain the rotate/inverse.

Missing Things

I think I’d have received more value had I not led with my questions. I’m sure these experienced individuals would have seen things in the code that I do not see. I think it would be better if I were to hold my questions to the end of the discussion. I was too much in a hurry, and I like to drive. Some would call that a flaw. I would not disagree.

Parent-Child and Update

Present in the model but not obvious is a design decision that objects will know their parents and that the object collection will be defined such that the more primitive objects get to update before the ones that depend on them.

Hill brought this up and suggested that a better design was needed (quite true, we’re just not there yet), and that one might prefer a design where the parents owned the children instead of the other way around. Then each object’s update would consist of updating itself and then calling update on its children. Very neat.

We noted, however, that if there are any children who depend on more than one object, there would have to be some scheme to ensure that all the parents were updated before the children try to update. We do not know if the situation will ever arise.

I’m inclined to stick with the current scheme, but the whole update process is very much a work in progress, with simple collections where one might expect a more robust structure.

Mutability

We all agree that immutable objects are better. But none of us really cared about these ones. I think this is the biggest clue that I got about the impact of my leading with questions. I think that if people had been let to dig around on their own, there would have been more interest and more value for all of us.

Compute and Draw Together

We have all been taught and learned by experience that when an object has two distinct capabilities, there’s a good chance that we should have two objects. My component objects all suffer from this condition: each one computes its start and finish, and each one knows how to draw the picture that we use to see that things are working.

None of us was particularly concerned about this. The objects are small, no one wants to build a full graphical view stack for these little pictures, and the computing and drawing functions are generally distinct.

Overall Results

I definitely got value out of the session. I suspect that the rest of the gang would have enjoyed it more, and that I would have received better feedback, had I held my questions until the end, and done less explaining except in response to questions.

No, I don’t suspect that: I’m sure of it. I would have done better to be quiet and listen, maybe asking my questions later if the topics hadn’t arisen.

Live and learn. I’ve lived so long, and learned so little in some ways. Fascinating. I’ll try to do better later. I’ll ask for another review and try to be quiet except in response to questions.

Future

I think the code is on a path to improve some of the issues above, perhaps even the compute and draw being together, and almost certainly reducing some of the mutability. I think I could get rid of all of it but I’m not sure it’s worth it.

We’ll find out. Meanwhile, I’ll try shutting up. Maybe.

See you next time!