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 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
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
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.
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).
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
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
nodemethod fail. Weird. Why?
dbtable error is because the test previously did not
Everything. Now it does. The test overrode
Everything::logErrors() badly. After fixing that, the test
htmlcode test has the same error and the same fix.
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
nodemethod uses the weird “Did you load this other module?”
trick. Nevermind that. Just check
%INC if it’s really
All tests should now pass again (and
skips most of its tests… but that’s fine. I know and you know). They
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
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.