This 30-day project explores the refactoring of a legacy system. The Everything Engine is an aging software project that powers Perl Monks, Everything 2, and a few other websites. It suffers from poor design and maintainiability. Learn what it’s like to look over the shoulder of an experienced developer as he refactors, redesigns, and updates the code.
Improving the Node Object Hierarchy
What should I do today? I really dislike the half-object, half-not scheme of nodes. How about forbidding direct attribute access in preparation to make them actual Perl objects?
Sounds good to me.
What needs to happen? Mostly generating accessors. It might be necessary to create some of these at startup time as there aren’t necessarily Perl modules for each node in the system. I’ll deal with that later. Even Ruby on Rails requires some model support, even if it is only two lines.
Perhaps the right idea is to use Class::Std to build these
objects. The latest versions should work with mod_perl.
I’ll start with the user node, as it’s short.
Do the tests still pass? Yes. Good.
Hm, actually with the inheritance not set in these nodes, it’s
really difficult to make accessors. user inherits from
node, so node has to set the data properly.
sigh. That means I have to make these objects into proper objects,
in the Perl sense, first. Wow. Okay. What’s the inheritance hierarchy?
To figure that out, I looked in the ecore repository, under
nodes/nodetype/. All of the XML files there list this information
under the extends_nodetype field. (Wow, we sure didn’t know
what we were doing with XML back in those days — you can tell because we
used the DOM. Urgh.) Just about everything extends node, which
is nice.
Still, there are 28 nodetypes and only 15 Perl modules. That means at some point I need to create 13 more. For now, I’ll mark the inheritance correctly in the existing nodetype classes.
Hmm, dbtable has a list of SQL keywords in it. Isn’t there
a Perl module that already has this?
I’ll have to go back and add a test for each of these to check the inheritance. That’s easy though.
Ugh, have I mentioned how much I hate having the XML-rendering code in specific nodes? Factory that out later!
I’ve also taken the opportunity to touch up the copyright notices and remove some useless POD and comments (and commented-out code).
Hmm, setting has some useless methods. Whack! Wow, the
tests fail now… that’s because they’re reasonably awful. (I wrote them; I
can say.) They’ll probably remain failing, so fixing them is the job for
the next day. Fortunately, it’ll be a lot easier with
Test::MockObject.
There’s no test for theme. Easy to add.
I just finished editing all of the code for these nodes. All of the
tests still pass (though I made the setting test skip):
$ ./Build test
Oops, they don’t. The tests for dbtable,
htmlcode, nodeball, nodegroup, and
nodemethod fail. Weird. Why?
The dbtable error is because the test previously did not
load Everything. Now it does. The test overrode
Everything::logErrors() badly. After fixing that, the test
passes. Next!
The htmlcode test has the same error and the same fix.
The nodeball test is different. Its problem is that the
test for module loading from the class is awful. That’s fine. It’s
mostly unnecessary anyway. (Have I mentioned that if these classes are
really truly classes, someday they can use Test::Class and get
rid of a lot of duplicate code?)
nodegroup has the same logErrors() problem.
Fixed.
nodemethod uses the weird “Did you load this other module?”
trick. Nevermind that. Just check %INC if it’s really
important.
All tests should now pass again (and setting still
skips most of its tests… but that’s fine. I know and you know). They
do.
Now to test the inheritance.
Hmm, along the way of editing these files, I noticed that t/Node/nodebase.t didn’t actually test a node. It was just a copy of t/NodeBase.t. Weird. Remove it! (That’s change #832.)
During this, I added the theme test. Everything all passes.
That means I can check this in now (#833) and fix the setting
tests on day four.
I don’t feel like I made as much progress today. That’s probably a sign of ill-factoring in the design. Of course, cleaning up scary code always takes longer than you expect, and there are a lot of files here. Fortunately, there are also more tests now.

You might want to give Moose at look instead of Class::Std. It does not bring along all the baggage of inside-out objects (required DESTROY calls, etc.), and will handle your attributes and accessor generation for you. It also will (very shortly) have full Perl 6 style Roles, and of course has all the .meta goodness of Class::MOP backing it up.
I do like inside-out objects, but plan to look at Moose in the near future. If possible, I'd like to turn these objects into more-or-less record/model objects, so it's not clear if I need much beyond accessor generation.