During the development of the soon to be released Restructure101, we used JUnit as a test harness to determine if Restructure101 could in fact do what it was being designed to do. In short, we wanted to see how easily one could remove tangles and improve its architecture with zero knowledge of the code, and, where possible, avoid breaking its API.
Unfortunately JUnit is mostly an API so it ended up primarily as a product management/test exercise. But the results warrant a blog post as it did demonstrate very clearly how in a very short space of time one can make some fairly significant improvements to the architecture of your code.
We fed our results back to JUnit‘s David Saff and Kent Beck. Here’s what they had to say:
“The finishing touches are now going into JUnit 4.9. Many of the suggestions from Restructuring101 were good, and we would have implemented them if JUnit were an internal library.
Unfortunately, a majority of classes in JUnit have many more external than internal uses, and it’s hard to justify breaking or deprecating legitimate clients to improve internal structure. Lessons learned for next time, however.”
“Lessons learned” indeed – demonstrating the importance of monitoring your software architecture from the get-go!
So, if you are in the position of starting out, be sure to use a tool like Structure101 to keep your architecture in check. Or if you already have a well established code base whose architecture is not where you’d like it to be, read on and see how you can get back on track in a surprisingly short space of time.
Summary first . . .
Because this is quite a long blog, let’s see why you should persist with it!
This is how the structure looked when I started out. The red-borders indicate items within TANGLES:
And these were the violations based on there target architecture:
WHEREAS here is the improved structure of the updated source code, which is TANGLE-FREE:
And violation free:
The time taken to perform/figure out refactorings in Restructure101 (which involved trying out various scenarios before deciding to document this approach was about 2 hours).
The time taken to manually perform (and in one case alter) refactorings in Eclipse was 40 mins. approx. And this included taking screenshots, etc, as I went along. I believe that if I had known the code base better, both the refactoring-planning and the refactoring-implementation processes would have been much quicker.
Tools used: Restructure101 v1.0 beta 1, Structure101 v3.3 and Eclipse 3.4.1 only!
(Note, this exercise was carried out using Restructure101 beta 1 and some widgets have been enhanced in more recent builds of Restructure101. Hence the difference between some screenshots here and those you might see in a later build of Restructure101)
And so, here’s how it was done…
Starting out . . .
First I downloaded the JUnit 4.8.2 source code, and then compiled it in Eclipse.
Next, after I pointed Restructure101 at the generated class files, I quickly descovered that there were tangles at three levels in the source code:
- At the root between packages junit and org
- Within package org.junit
- Within package org.junit.runner
The red boxes on our LSM (levelized structure map) showed the feedback items involved the tangles:
And so I decided to attempt to:
- Solve these tangles using Restructure101, generating an action list in the process
- Create a tangle-free target architecture diagram using Structure101, and publish it, so that I could monitor improvements as actions were implemented
- Implement the actions recorded in Restructure101 on the JUnit 4.8.2 source code, using Eclipse to implement the changes
And doing all of this should give me a tangle-free code base!
Step 1: Solving the root tangle between junit and org (by moving classes)
When I drilled down into package junit, I saw many classes involved in the tangle, denoted by the red border:
But selecting to display dependencies from junit hinted that perhaps junit should be beneath org in the LSM, as it appeared to be used by org more often that it used org:
So, I thought that if we turn this concept around and move the classes NOT involved in the tangle (or more specifically, only those that are USING org) then junit should drop to the bottom of the LSM.
To do this I performed the following actions:
(Note that these moves could have been performed by selecting all three nodes and moving (via drag and drop) in one action. But, for the purpose of this exercise, I performed actions individually for greater clarity)
And so, after doing this, tanglicity went down from 100% down to 71%. And we ended up with a LSM like:
Much more attractive already!
Step 2: Solving the tangle within org.junit.runner (by creating an interface)
Focusing on this, I quickly saw the problem was that two classes in the notification package were using the Result class in the runner package.
(Note the JUnitCore, Request and Computer red-bordered items were involved in a tangle OUTSIDE of the runner package. I addressed these later in Step 3)
And so we had a trickier situation in this case, since we were constrained by existing potential user/API usage.
To avoid moving a class, in this case, as one solution might be to simply move the notification classes up to the runner package, I attempted to interface the Result class.
I simulated this by performing the following actions:
(Note again that these moves could have been performed in one action via multiple drag and drop, but I chose to do them individually for this exercise)
After the application of these actions the tanglicity did not decrease as a result, as it was too low in the hierarchy to have an impact. But, nevertheless, the structure was now cleaner within the runner package:
Step 3: Solving the tangle within org.junit
From the following screenshot it was easy to see why I left this till last. It was not going to be straightforward.
But the first thing that jumped out at me, and after reading some documentation about the JUnit interface, was that the internal package seemed to be causing a lot of confusion – and FOR THIS EXERCISE I THEN ASSUMED THAT I COULD REFACTOR THIS WITHOUT RESTRICTION, OR THE NEED TO MAINTAIN ANY API.
So, firstly, I flattened-to-classes the internal package, which meant I moved all classes “recursively within” the internal package to the internal package, and then purged the old package hierarchy under internal. Restructure101 offers a single action to do all of this for you.
Next I moved classes to where I thought they belonged, moving classes up and down the hierarchy into existing packages, until I had a clean internal package.
While in the process of doing this, I took a screenshot after eleven actions:
And, already, it looked like the structure was improving. The runner package was now above internal (which made sense to me at least), matchers had dropped down, experimental was no longer part of the tangle, and the Assert class now had all of its collaborative classes in the same package as it – and hence was no longer part of the tangle. (Note that Assert could not be moved for API reasons)
So – AS A FLASHBACK – I compared this to the structure we started out with:
Looked better than this, didn’t it?
And so, back to restructuring!
Next I noticed that the remaining offenders in the internal package were either used by, or used, by either or both the runners and runner packages:
And so, things got a little more difficult again.
Then, upon investigating more, I noticed that InitializationError was causing a lot of pain!
So, I decided to move it to a lower package.
And when I decided to move it to the internal package I got a message saying – hey – this package has class with this name already.
So, I guess I’d just discovered some duplicate code.
Then, I renamed the guy I wanted to move, and moved it (and you could add note in Restructure101 to tell the developer to merge these classes if you wanted).
Then things looked better again!
On investigating further, I noticed that the runners.model package seemed to be causing a lot conflict, and I wondered if it was in the correct location?
Perhaps model should be within runner?
I thought so!
Next I went back to tidying the internal package. And it very quickly came right! When moving classes from the internal package, I created new internal packages in the target (existing) packages so as I’d know where they came from!
And I ended up with this (after sixty nine actions):
At this point, we had only four offending classes remaining – the four items with a red border.
And this could be narrowed down to two issues:
1) Should the BlockJUnit4ClassRunner class be lower in the hierarchy, like the JUnit38ClassRunner (which was not creating any problem)?
2) Should Suite be in the runner package?
To fix these remaining issues I ran the risk to breaking an API (somewhere), but I went ahead anyway, and as a result of moving classes (from runners) to where I thought they belonged, I also had to move the IMethodRule interface from the rules package (as this was being used by BlockJUnit4ClassRunner):
And then we were tangle free, and our chart showed a fully structured code base:
And all this after seventy four actions.
Step 4: Publish Actions and Create a Diagram
Next, to prove that we could impose these changes on our code base, I published these actions to a repository so that our Eclipse plugin could pick up the actions a developer needs to implement the refactorings:
And when I started Eclipse, and configured it to point to that repository, I saw the action list TO DO.
I also created a diagram in Structure101 of the target architecture, and published it, so as I could see the structure coming right in the plugin as I refactored the source code:
And so I began ….
Step 5: Refactoring in Eclipse guided by the action list
First, I created a new package and then moved the three junit4 classes using Eclipse move:
Once I did this, the diagram refreshed in Eclipse, and I saw an instant improvement (violations were reduced from fifty-five to thirty-eight).
And so I continued in this vein… making my way through the actions.
However, while solving tangle two, I noticed that extracting the interface was not enough, or at least I needed to tweak the refactoring so that ResultImpl was renamed to Result (maintaining the API), the new Result interface was renamed to IResult, and was then moved to notification. And this actually seemed a better result (pardon the pun).
So, while my proposed refactoring in Restructure101 didn’t get it 100% correct (and remember I chose what course to take, not Restructure101), I was 90% there, and with some tweaking within Eclipse (I didn’t have to go back to Restructure101 for this) it worked out fine.
Next, I tackled the big one!
And, less than ten minutes later, almost two-thirds of the way through the actions, I took a look at the code base as it was (loading it in Restructure101):
And it looked very like our proposed model earlier on, didn’t it!
So, I was going well…
And when I checked the target architecture in the plugin, its number of violations was coming down nicely:
From thirty-eight to nine.
And less than ten minutes later, with only five actions remaining, the as-is structure was:
And one minute later I had a code base like:
And in Restructure101 the code base was shown to be complexity free:
And this was without the need for any actions in Restructure101.
And our target architecture in the plugin also showed this (zero violations):
Job done.
Leave a Reply