I Built His Cage

Greetings ladies and gentleman, your favorite bitter clinger is here and back for 2009 with new material. Watch for a new episode of Nowhere To Run this Saturday.

I arrived back at work, after a two-week vacation, fearing the worst: public static everything, classes that I had carefully crafted to have one responsibility now having two or more, bloody red bars all over my unit tests, long parameter lists.

The damage wasn’t as widespread as I imagined. I wasn’t the only one on vacation, after all. There were scattered unit test failures. There is only a couple of slightly long parameter lists. There was even one constants class inheriting from another constants class, and based on its usage, I actually don’t have a problem with this.

There was, however, a public static utility class, intended to for common use.

As I started enumerating my complaints, any number of analogies flashed through my mind:

• “suit up, marine, and take your shotgun with you”
• “well, I need my drill bit and welding mask, because this building has a crack in it”
• “this truck needs an oil change”
• “you will never escape me, your soul will be mine!“

I admit being partial to the last one.

Amazing things will happen here soon…

I am on Team A. Another developer is on Team B. Both of our teams are working separate, but related, projects. We share an entity object from which we both inherit.

Due to a previous project, the persistence code of which we are heavily re-using, our entity objects contain, in addition to the typical private member variables, an inherited HashMap. It maps property names to property values.

You’re asking, like I did, “why, oh why?” and the answer is “because, oh because certain properties of the entities need to be indexed, in the database, in a very special way for performance reasons, and this HashMap is how it is accomplished.” This blog will refer to such properties as Special.

When an entity is saved, this HashMap is populated, at some point before the actual saving. When an entity is loaded, this HashMap is populated as well, and when a Special property is accessed, it is, at some point, fetched from the HashMap.

Very well. Now, each team will need to specify which of their project-specific properties are Special. But since we both share a common base class, with its own properties, we’ll also need to determine which of our inherited properties are Special.

This also means that both teams have to agree on how to share the logic that moves to and from HashMap and logical property.

Counsel, make your opening statement

Three developers spent a total of twelve man-hours gawking over five equally-hated solutions to this problem:

1. Team B’s way: accessors of Special properties actually retrieve the values from the HashMap. This is where the public static tragedy came from. Team B wanted to throw the Special property field names, common to both teams or not, along with a couple of public static methods, into one shared class.

   I hate it. Helper classes suck:

   Because most of us in the OO world came out of the procedural programming world, and the notion of functional decomposition is so easy that we drop to it when we come across an algorithm that doesn’t seem to “fit” into our neat little object tree, and rather than understand the needs, analyze where we can get the best use of the technology, and place the method accordingly, we just toss it into a helper class. And that, my friends, is laziness.

   Add to this that not fielding the Special properties makes the code harder to understand and gives the entity one more responsibility that it shouldn’t have.

2. Team A’s (a.k.a. mine) original design: field the Special properties and write a common class that handles the translation to and from private members and the HashMap. Each team delegates to this common class to handle shared Special properties.

   Team B hates it. They claim this involves too many classes and too much “similar code”, or code calling the same method with a few different parameters. They allege this to be duplication. Team B wants everything having to do with Special properties to be in one class and one class only.

3. Compromise 1: field the Special properties and write a single common class that handles all Special properties, common or not. Put all field names, field types, and accessor method names in this class. Switch on the passed-in entity’s actual type. Use reflection to get and set Special property values.

   I hate it. Reflection sucks. It is slow and hard to read. I don’t like all of the information this class has either. I loathe having to write Java code that has instanceof.

4. Compromise 2: field the Special properties and write a single common class that handles all Special properties, common or not. Put all field names and field types in the project-specific entities themselves.

   I hate it. This involves entities being concerned with something that is persistence-related. It also involves entities having methods to get and set Special properties by name, and since Java lacks function pointers, you’ll have the same big if-else statement repeated twice.

5. Compromise 3: field the Special properties and write a single common class that handles all Special properties, common or not. Put some of the field names, field types, and accessor method names in this class, and the rest in the project-specific entites themselves.

   I hate it. It has the disadvantages of the first two compromises with the benefits of neither.

He’s no where to be found, sir

What I found to be so interesting about this epic debate is how deep the differences ran. This wasn’t a mere conflict of which solution would work. All five of them would get the job done. This was a clash of ideology.

• Team B thinks that similar code is duplicated code. Here is an example of similar code:
  
  foo.bar(x, y, z);
  foo.bar(x, a, b);
  
  I don’t agree. I take a strict copy-paste view of code duplication.

• Team B thinks centralizing the Special properties makes it easier to maintain. Both teams edit one source file for their Special property needs.

  I don’t agree. This makes both teams, and those that may come along tomorrow, dependent on this common code, and if one team’s requirements change, all teams’ code that depend on this code have to be re-tested.

  This is endemic of the design of the mess I alluded to in Chaos Crusher. It also flies in the face of conventional (or maybe it’s not) thought on package design: lower-level packages should be more stable than higher-level packages.

• Team B distinguishes between the non-Special properties of an entity and the Special properties of an entity.

  I don’t agree. And you shouldn’t be surprised that a domain-driven design practitioner is property-blind.

• Team B had no qualms about writing a class with only public static methods.

  There is little you can do to convince me that writing public static methods like this has a whiff of OO-ness. I consider it beneath an Anemic Domain Model.

• Team B thinks reflection is easy.

  I don’t agree. It’s hard to read. I don’t want to have to run the code to figure out what it does. I also don’t want run-time exceptions to replace compile-time errors.

I can’t believe I get paid for this

Having decided that all solutions equally sucked, both sides succumbed to attrition and re-delegated the choice of the solution to me. And I went with our (read: mine) original design. After twelve man-hours of talk, the kind of arguments that I have in my head with myself, implementation and testing took all of ten minutes.

I can’t believe I get paid for this.

Announcer: You’re reading the EIP web-ring.