Keep your ovens clean

Let’s assume for a moment that you are a baker, producing different types of pastries in your small bakery. The production process is always the same: prepare the dough, put it in the oven, wait some time and retrieve the most delicious buns or bread. If we can abstract the real baking process to these steps, it’s the same as with software: prepare the sourcecode, put it in the compiler, wait some time and retrieve the most delicious binary or executable. There is only one difference: The oven of the baker is a self-contained, closed system, while our compilers require a distinct system setup around them in order to produce anything edible. The oven is independent from the kitchen around it, the compiler is depedent on the environment. To finish the analogy, what would a baker say if he can’t bake bread in his oven unless he nurtures a certain type of yeast in his kitchen?

A most unpleasant case

While developing a platform dependent application recently, we met a most unpleasant case of build dependency on a third-party library. It was an old dynamic link library (DLL) that requires registering in the windows registry. There was no other way than to register the DLL using the regsrv32 utility. If you didn’t do this, the build process would abort with an error stating unmet dependencies. If you ran the resulting program on a machine without registered DLL, it would crash with a runtime error complaining about the missing registry entry. And by the way, there are two totally independent regsrv32 utilities on a 64-bit windows system, one for 32-bit and one for 64-bit registrations. No, the name of the latter one isn’t regsvr64, that would be way too easy.

We accepted the fact that you need to prepare your system if you want to run the program, but we quarreled a lot with the nuisance that you need to alter your system just to build the software. This process of alteration is called snowflaking in the DevOp mentality and it’s not a desired activity. We would need to alter every build machine in our continuous integration cluster that comes into contact with the project. And we would need to de-snowflake them again afterwards, because this kind of tinkering adds up to inscrutable side-effects very fast.

A practicable workaround

We found a way around the abovementioned snowflaking for our build servers. It’s not a solution, it’s only a workaround, as it solves the immediate problem but produces some lesser problems on the way. Let’s look at what we did.

At first, our situation could be described with this module diagram:

dependency1We couldn’t modify the problematic DLL itself, it was a given binary. But we could wrap it in our own DLL. Wrapping less pleasant things into something you can control is a proven technique even in baking, by the way. We now had a system layout that looks like this:

dependency2Nothing gained so far, just that we now have a layer outside our system that can provide the functionality of the DLL and is actually under our control. The wrapper really does nothing on its own but to forward each call to the DLL. To profit from this indirection, we need to introduce another module, like this:

dependency3The second module provides the same interface as the first, but does nothing, not even forwarding anywhere. It’s a complete stub, just there to be uncomplicated during the build process. The goal is to build the system using this “empty” DLL and then replace it with the “problematic” DLL afterwards. The only question is: how do we build the problematic DLL? Here’s the workaround part of the solution: We actually had to compile the problematic DLL on a snowflaked system and add it to the project repository. Good thing our target system’s specification is known, so we only need to do this for one platform. Because we are reasonably sure that the DLL interface will not change over time (it had every opportunity in the last ten years and didn’t use it), we can assume that the interface of our two wrapper DLLs also won’t change. So it’s not too problematic to check in a precompiled binary that needs to satisfy an interface that’s reproduced with every build cycle. Still, we need to keep an eye on the method signatures of our two wrapper DLLs. If one of them changes, the modification needs to be replicated on the other wrapper, too. It’s a classic duplication.

When we balanced the duplication in the interfaces of the two wrapper DLLs against the snowflaking of every CI and developer machine, we found our aversion against snow outweighing the other negative aspects. Your mileage may vary.

Conclusion

We kept our build ovens clean by introducing a wrapping layer around the problematic depedency and then using the benefits of indirection by switching to a non-problematic stub during the build cycle. The technique is very old, but still use- and powerful.

A tale of scrap metal code – Part I

This is the beginning of a long tale about an examined software project. It is too long to tell in one blog post, so I cut it in three parts. The first part will describe the initial situation and high-level observations. The second part will dive deep into the actual source code and reveal some insights from there. The third part wraps everything together and gives some hints on how to avoid being examined with such a negative result.

First contact

We made contact with a software product, lets call it “the application”, that was open for adoption. The original author wanted to get rid of it, yet it was a profitable asset. Some circumstances in this tale are altered to conceal and protect the affected parties, but everything else is real, especially on the technical level.

You can imagine the application as being the coded equivalent to a decommissioned aircraft carrier (coincidentally, the british Royal Navy tries to sell their HMS Invincible right now). It’s still impressive and has its price, but it will take effort and time to turn it around. This tale tells you about our journey to estimate the value that is buried in the coded equivalent of old rusted steel, hence the name “scrap metal code” (and this entry’s picture).

Basic fact

Some basic facts about the application: The software product is used by many customers that need it on a daily basis. It is developed in plain Java for at least three years by a single developer. The whole project is partitioned in 6 subprojects with references to each other. There are about 650 classes with a total of 4.5k methods, consisting of 85k lines of code. There are only a dozen third-party dependencies to mostly internal libraries. Each project has an ant build script to create a deployable artifact without IDE interference. On this level, the project seems rather nice and innocent. You’ll soon discover that this isn’t the truth.

Deeper look

Read the last paragraph again and look out for anything that might alert you about the fives major failures that I’m about to describe. In fact, the whole paragraph contains nothing else but a warning. We will look at five aspects of the project in detail: continuity, modularization, size, dependencies and build process. And we won’t discover much to keep us happy. The last paragraph is the upmost happiness you can get from that project.

Feature continuity

You’ve already guessed it: Not a single test. No unit test, zero integration tests and no acceptance test other than manually clicking through the application guided by the user manual (which we only hoped would exist somewhere). No persisted developer documentation other than generated APIDoc, in which the only human-written entries were abbreviated domain specific technical terms. We could also only hope that there is a bug tracker in use or else the whole project history would be documented in a few scrambled commit messages from the SCM (one thing done right!).
The whole project was an equally distributed change risk. The next part will describe some of the inherent design flaws that prohibited changes from having only local effects. Every feature could possibly interact with every other piece of code and would probably do so if you keep trying long enough.
It’s no use ranting about something that isn’t there. Safety measures to ensure the continuity of development on the application just weren’t there. FAIL!

Project modularization

The six modules are mostly independent, but have references to types in other modules (mostly through normal java imports). This would not cause any trouble, if the structure of the references was hierarchical, with one module on top and other modules only referencing moduls “higher” in the hierarchy. Sadly, this isn’t the case, as there is a direct circular dependency between two modules. You can almost see the clear hierarchical approach that got busted on a single incident, ruining the overall architecture. You cannot use Eclipse’s “project dependencies” anymore, but have to manually import “external class folders” for all projects now. The developer has forsaken the clean and well supported approach for a supposable short-term achievement, when he needed class A of module X in the context of module Y and didn’t mind the extra effort to think about a refactoring of the type and package structure. What could have been some clicks in your IDE (or an automatic configuration) will now take some time to figure out where to import which external folder and what to rebuild first because of the cycle. FAIL!

Code size

The project isn’t giantic. Let’s do some math to triangulate our expectations a bit. One developer worked for three years to pour out nearly 90k lines of code (with build scripts and the other stuff included). That’s about 30k lines per year, which is an impressive output. He managed to stuff these lines in 650 classes, so the average class has a line count of 130 lines of code. Doesn’t fit on a screen, but nothing scary yet. If you distribute the code evenly over the methods, it’s 19 lines of code per method (and 7 methods per class). Well, there I get nervous: twenty lines of code in every method of the system is a whole lot of complexity. If a third of them are getter and setter methods, the line count rises to an average of 26 lines per method. I don’t want every constructor i have to use to contain thirty lines of code!

To be sure what code complexity we are talking about, we ran some analysis tools like JDepend or Crap4j. The data from Crap4j is very explicit, as it categorizes each method into “crappy” or “not crappy”, based on complexity and test coverage (not given here). We had over 14 percent crappy methods, in absolute numbers roughly 650 crappy methods. That is one crappy method per class. The default percentage gamut of Crap4j ends at 15 percent, the bar turns red (bad!) over 5 percent. So this code is right at the edge of insanity in terms of accumulated complexity. If you want to know more about this, look forward to the next parts of this series.

Using the CrapMap, we could visualize the numerical data to get an overview if the complexity is restricted to certain parts of the application. You can review the result as a picture here. Every cell represents a method, the green ones are okay while the red ones are not. The cell size represent the actual complexity of the method. As you can see, the “overly complex code syndrome” is typically for virtually all the code. Whenever a method isn’t a getter or setter (the really tiny dark green square cells), it’s mostly too complex. Additional numbers we get from the Crap4j metric are “Crap” and “Crap Load”, stating the amount of “work” necessary to tame a code base. Both values are very high given the class and method count.

All the numbers indicate that the code base is bloated, therefore constantly using the wrong abstraction level. Applying non-local changes to this code will require a lot of effort and discipline from an experienced developer. FAIL!

Third-party dependencies

The project doesn’t use any advanced mechanism of dependency resolution (like maven or ant ivy). All libraries are provided alongside the source code. This isn’t the worst option, given the lack of documentation.
A quick search for “*.jar” retrieves only a dozen files in all six modules. That’s surprisingly less for a project of this size. Further investigation shows some inconvenient facts:

  • Some of these libraries are published under commercial licenses. This cannot always be avoided, but it’s an issue if the project should be adopted.
  • Most libraries provide no version information. At least a manifest entry or an appended version number in the filename would help a lot.
  • Some libraries are included multiple times. They are present for every module on their own, just waiting to get out of sync. With one library, this has already happened. It’s now up to the actual classpath entry order on the user’s machine how this software will behave. The (admittedly non-present) unit tests would not safeguard against the real dependency, but the local version of the library, which could be newer or older.

As there is no documentation about the dependencies, we can only guess about their scope: Maybe the classes are required at compile time but optional at runtime? The best bet is to start with the full set and accept another todo entry on the technical debt list. FAIL!

Build scripts

But wait, for every module, there is a build script. A quick glance shows that there are in fact four build scripts for every module. All of them are very similar with minor differences like which configuration file gets included and what directory to use for a specific fileset. Nothing some build script configuration files couldn’t have handled. Now we have two dozen build scripts that all look suspiciously copy&pasted. Running one reveals the next problem: All these files contain absolute paths, as if the “works on my machine award” was still looking for a winner. When we adjusted the entries, the build went successful. The build script we had to change was a messy collection of copied code snippets (if you want to call ant’s XML dialect “code”). You could tell by the different formatting, naming and solution finding styles. But besides being horribly mangled, the build included code obfuscation and other advanced topics. Applied to the project, it guaranteed that no stacktrace from any user would ever contain useful information for anybody, including the project’s developers. FAIL!

Summary

Lets face the facts: The project behind the application fails on every aspect except delivering value to the current customers. While the latter is the most important ingredient of a successful project, it cannot be the only one and is only sustainable for a short period of time. The project suffers from the lonely superhero syndrome: one programmer knows everything (and can defend every design decision, even the ridiculous ones) and has no incentive to persist this knowledge. And the project will soon suffer from the truck factor: The superhero programmer will not be available anymore soon.

Prospectus

There are a lot of take-away lessons from this project, but I have to delay them until part three. In the next part, we’ll discover the inner mechanics and flaws of the code base.