We’ll explore aligning responsibilities in our little objects, and I even think we’ll rename one of them, now that we can see what it is.
We’re zeroing in on a structure for representing paths as combinations of other paths. We need this, in particular, for SLRR, where we create the path dynamically as we go. I think we will likely find it useful in other situations. But even if a thing is going to be used only once, I like to get its pieces and parts in the right place. It makes understanding easier, which makes usage and maintenance easier. And, not that I’d know anything about it, I am told that it makes debugging easier as well.
Before we dig in, let me write down here all the notes that I have on scraps of paper on my keyboard tray.
It’s that last item that I’m after in the title of this piece: “Where does this go?” Here’s the relevant code in Bezier:
Bezier = class()
function Bezier:linearized(n)
local beziers = self:partition(n)
return L_Bezier(beziers)
end
We have at least two issues here. First, though the object is named L_Bezier, it really has approximately nothing to do with Beziers. It is a path consisting of variable-length linear segments.
Currently it starts like this:
L_Bezier = class()
-- contains variable-length linear segments
-- typically produced from a bezier via `linearize`
-- but any old segments will really work if you can
-- get them in there.
function L_Bezier:init(beziers)
self._points = self:_create_points(beziers)
self._lengths, self._length = self:_create_lengths(self._points)
end
I left in the comment above, which is really just a note to myself, reflecting my uncertainty about quite what the responsibilities of the various objects should be.
A Bezier is a kind of thing for producing points, and it’s not even a kind of thing that we really like, because it doesn’t produce them in a nice linear fashion. That’s why we have the partitioning and the L_Bezier. So the Bezier object, it seems to me, should now know about the L_Bezier class. It doesn’t fit in with what Beziers know and do.
At the same time, the L_Bezier class is more than just a tool for dealing with linearized Beziers. It can deal with just about any kind of path. Properly defined, it would provide convenient methods for creating instances.
So … I think we want L_Bezier to produce something that it might reasonably know about. It knows how to split itself, providing two instances of Bezier. It seems to me that linearized should return an array of Bezier objects that add up to the original. That seems to me to be the proper domain of Bezier.
Enough chatter, let’s just do it and make things work. Here’s linearized now:
function Bezier:linearized(n)
local beziers = self:partition(n)
return L_Bezier(beziers)
end
So that’s easily broken in the direction we want. Just return the array from partition and be done with it. (We’ll have a refactoring opportunity coming up.)
function Bezier:linearized(n)
return self:partition(n)
end
We see the possible refactoring, collapsing linearized and partition into one method. Later: we are already on a quest and do not need a side quest. Test to see what breaks. I am not best pleased. This test fails:
function Tests:test_pathfinder_l_bezier()
local p0 = vector(000, 100, 0)
local p1 = vector(100, 100, 0)
local p2 = vector(100, -100, 0)
local p3 = vector(000, -100, 0)
b1 = Bezier(p0, p1, p2, p3)
lb1 = b1:linearized(3)
p0 = vector( 000, -100, 0)
p1 = vector(-100, -100, 0)
p2 = vector(-100, 100, 0)
p3 = vector( 000, 100, 0)
b2 = Bezier(p0, p1, p2, p3)
lb2 = b2:linearized(3)
local pathfinder_table = { {lb1:length(), lb1}, {lb2:length(), lb2}}
local pathfinder = PathFinder(pathfinder_table)
local half = lb1:length() / 2
local near_side = pathfinder:at(half)
local b_near_side = b1:at(0.5)
local far_side = pathfinder:at(3*half)
local b_far_side = b2:at(0.5)
self:verify_vectors(near_side, b_near_side)
self:verify_vectors(far_side, b_far_side)
end
The failure is on the would-be call to length(). It’s happening because we expect linearized to return an L_Bezier and it returns and array of Beziers. I’ll change those two linearized lines. I think I want a creation method on L_Bezier, like this:
function Tests:test_pathfinder_l_bezier()
local p0 = vector(000, 100, 0)
local p1 = vector(100, 100, 0)
local p2 = vector(100, -100, 0)
local p3 = vector(000, -100, 0)
b1 = Bezier(p0, p1, p2, p3)
lb1 = L_Bezier:from_beziers(b1:linearized(3))
p0 = vector( 000, -100, 0)
p1 = vector(-100, -100, 0)
p2 = vector(-100, 100, 0)
p3 = vector( 000, 100, 0)
b2 = Bezier(p0, p1, p2, p3)
lb2 = L_Bezier:from_beziers(b2:linearized(3))
-- etc
This will fail for want of that method.
bezier_dev.lua:427:
attempt to call missing method 'from_beziers' of table.
Tests:test_pathfinder_l_bezier
OK, now we can write that. Fact is, we could have just created the L_Bezier directly there. That would have been a smaller step but this is OK.
L_Bezier = class()
function L_Bezier:from_beziers(bez_array)
return self(bez_array)
end
function L_Bezier:init(beziers)
self._points = self:_create_points(beziers)
self._lengths, self._length = self:_create_lengths(self._points)
end
That works just fine. Commit it. Push it.
I’ve added these items to my scrap of paper:
Let’s work on the messy creation. Here’s the current code of interest:
function L_Bezier:from_beziers(bez_array)
return self(bez_array)
end
function L_Bezier:init(beziers)
self._points = self:_create_points(beziers)
self._lengths, self._length = self:_create_lengths(self._points)
end
function L_Bezier:_create_points(beziers)
local points = {}
for _, bezier in beziers do
table.insert(points, bezier.p0)
table.insert(points, bezier.p1)
table.insert(points, bezier.p2)
end
local final_point = beziers[#beziers].p3
table.insert(points, final_point)
return points
end
function L_Bezier:_create_lengths(points)
local lengths = {}
local total_length = 0
local previous = points[1]
for _, pt in points do
local len = previous:dist(pt)
total_length += len
table.insert(lengths, len)
previous = pt
end
return lengths, total_length
end
_create_points would be better named to refer to Bezier, since it is dealing with Beziers as its input. Each bez has four control points. Since they are from a partition, the fourth control point of bez N is equal to the first control point of bez N+1. So the _create_points is scarfing out just the first three control points of each bez, plus the fourth of the last.
What is this code doing? It’s actually creating a “polyline”, a path through the points from beginning to end. Let’s rename this method _create_polyline. Easily done. Test. Green. Commit and push.
I just noticed a bunch of commented out L_Bezier tests. I must have done that while focused on some specific thing and I forgot to uncomment them. This is why I gave myself ignore, but I was too lazy to change the all.
Uncomment the tests. Run them, expecting chaos. Chaos arises as predicted. Most of it is resolved by calling from_beziers as needed. Still two failures, both looking like this:
attempt to perform arithmetic (div) on function and number. Tests:test_l_bezier_interpolation
function Tests:test_l_bezier_interpolation()
local b = self:sample_bezier()
local lb = L_Bezier:from_beziers(b:linearized(0))
-- lb:dump()
local sqrtby2 = 100*math.sqrt(2) / 2
pt = lb:point_at_distance(lb.length/2)
self:assert_nearly_equal(pt.x, 550, 0.01, "x")
self:assert_nearly_equal(pt.y, 500, 0.01, "y")
end
Right. length became a function yesterday. Fix those. Darn. Forgot to change the dot to colon:
pt = lb:point_at_distance(lb:length()/2)
Same change in the other place. Back to green. Commit, push, hope no one noticed.
I resolve to use the ignore feature despite its slight inconvenience, so that I’ll be aware of such thing. Might even raise the priority of color in the test pane.
Where we we? Oh, right, improving the creation. We now have the method create_polyline, which accepts an array of beziers. Creation looks like this:
function L_Bezier:from_beziers(bez_array)
return self(bez_array)
end
function L_Bezier:init(beziers)
self._points = self:_create_polyline(beziers)
self._lengths, self._length = self:_create_lengths(self._points)
end
Now I think it makes perfect sense to create an L_Bezier from a polyline, so let’s create the polyline in from_bezier and have init expect a polyline, like this:
function L_Bezier:from_beziers(bez_array)
return self(self:_create_polyline(bez_array))
end
function L_Bezier:init(polyline)
self._points = polyline
self._lengths, self._length = self:_create_lengths(self._points)
end
Now it is interesting, I claim, that in from_beziers, self is actually the class L_Bezier, not an instance. Lua doesn’t care, and neither do we, except that create_polyline dare not really set anything into self, as it is a class method. Same for from_bezier.
I move them to the top of the class and put a comment at the top just as a reminder.
I think it has become obvious what this class should be called: PolylinePath. It has Path behavior and is based on a polyline. Change everything. Global replace works. Tests are green. Now I want to change the names of the tests and variables referring to the new class name, for example:
function Tests:test_l_bezier()
local b = self:sample_bezier()
local l_bezier = PolylinePath:from_beziers(b:linearized(0))
local lengths = l_bezier._lengths
local points = l_bezier._points
self:assert_equals(lengths[1], 0)
self:assert_nearly_equal(lengths[2], 141.42, .01)
self:assert_equals(points[1], b.p0)
self:assert_equals(points[4], b.p3)
end
I even go through and find a lot of lb kinds of variables that I rename, for example:
function Tests:test_polyline_path_create_lengths()
local b = self:sample_bezier()
local pp = PolylinePath:from_beziers(b:linearized(3))
local new_points = pp:_create_polyline(b:partition(3))
local new_lengths = pp:_create_lengths(new_points)
self:assert_equals(new_lengths, pp._lengths)
end
Green. Commit and push: rename l_bezier to polylinepath.
I think we’ve done nicely with PolylinePath now. By simplifying it and renaming it, we have made it more general. We don’t add features to get generality: we remove constraints. Much better way.
I’m left with the admonition to “converge linearized and partition”. It is clear that we are partitioning, not linearizing. Change all the calls to linearized to call partition instead. Green commit: remove linearized, use partition.
That’ll do, pig, that’ll do. We have resolved the big “should” question, coming up with a better division of responsibilities between Bezier and the newly-named PolylinePath. We found some tests that some tiny fool had commented out and fixed them up to run. And aside from a moment’s confusion due to not changing a dot to a colon, we did it all without panic or confusion.
A good morning. Best to back away slowly.
Safe paths!