It has been already some time since I started to oppose to idea of the Best Practice. Somehow it felt like an excuse to avoid thinking and using out-of-box solution. But I could not express my concerns well enough for the Best Practice Seekers and Followers to even start doubting it.

Now, I can use very good article Death to Best Practices written by Ted Neward. I encourage everybody to read it as well as an article linked there, but I offer my favourite part:

The point is, best practices just don’t exist. They are an attempt to take a solution to a problem out of the context and apply them across the entire spectrum, and that essentially invalidates the entire thing. A solution is only useful when considered in context.

Don’t believe me? Consider this challenge: when is it not a best practice to breathe? When you’re under water, of course.

(Unless you’re a fish. Or a frog. Or maybe a prince turned into a frog.)

Point is… context matters, folks.

I am looking forward for comments saying you actually can breathe under water, you just need to... But that would be only cognitive dissonance at work.

There is a value in the best practices, I have never doubted it, but I always suggested to use name good practices to show they are not universal truth. IMHO their value is in learning how experts solve particular problem in particular context, allowing to think whether it is a good match in our context, discussing differences in contexts and to think whether we could change our assumptions about problem we are trying to solve.

I have touched this topic before in Giving gifts versus conceiving rules and also in Is Javadoc useful?

More than year ago I was assigned to different project. To my delight team was writing not only unit tests, but also end-to-end tests. Every test class undeployed all plugins and components from server, reconfigured environment and deployed tested artefacts (not necessarily the same combination). Fantastic, tests were self-contained and independent.

But there were problems. Tests were slow and redeployment step was unnecessary in many cases, because all needed artefacts for subsequent test were already deployed and no changes were actually needed. Additionally all tests performed test setup in the old fashioned procedural way, describing in all gory details how to achieve desired state.

It could look like following code (comments are place holders for real functionality I am not going to show here).

@BeforeClass public static void beforeClass() {
    TestingUtil util = // get it ...

    util.undeployAllComponents();
    util.undeployAllPlugins();

    // initialize Database
    util.deployPlugin("daos.jar");
    util.deployPlugin("router.jar", routerParams);
    // some necessary tasks
    util.deployComponent("authenticator.jar");
    util.deployComponent("decisions.jar");
  
    User userA = // prepare user profile
    // ...
    // save user profiles
}

Instead of comments there were many lines of other code. Number of deployed components was varying between tests and also their order was not always the same (when it did not matter). Additionally there were 2 environments thus plugin router was usually unclosed with if-else together with varying DB setup. It was rather hard to track down what exactly is going on, but after some time we got used to it. If only it was not duplicated 17 times and every time it looked differently (nearly every test tested different component, but nearly all components depended on other components). Occasionally a new dependency was introduced and we needed to modify all those tests and add new one in correct place, unless it was not needed for particular test. It was messy.

Solution

I don't remember any more if I was begging our team leader to give me time to fix it or he asked me to do so, but I got the chance to find correct abstraction for this problem and to remove duplication.

I've realized our existing approach mixed together:

• dependency management
• description of scenario being tested
• deployment tasks
• varying environment
• preparation of data in database

So I have created following:

• Artefact knows what it is (plugin, component) and what other artefacts it depends on. There are subclasses for every component and plugin in order to encapsulate and reuse that knowledge.
• Scenario collects Artefacts that need to be deployed and can iterate through them in different ways (in deployment order, in undeployment order). It got more responsibilities later so I have split it to 2 separate classes.
• ScenarioDeployer is responsible for coordination of whole redeployment of scenario
• Deployer knows what environment it is deploying to and how to perform it.
• A hook for inserting custom code (e.g. for DB) between components are undeployed and deployed.

I have decided to push as much responsibilities as I could into individual objects. In this case the key was to give Artefact responsibilities of knowing what it depends on and how to deploy itself (by delegating to Deployer). Alternative with testing superclass having different methods doing deployment led to original mess. Alternative with static methods would fail too. What we've got was following:

@BeforeClass public static void beforeClass() {

    // initialize Database
    Scenario scenario = new Scenario();
    scenario.add(new AuthenticatorComponent());
    scenario.add(new DecisionsComponnent());
    createScenarioDeployer().redeploy(scenario);
  
    User userA = // prepare user profile
    // ...
    // save user profiles
}

Notice it does not mention anything about how to undeploy artefacts or which ones to undeploy. It will happen inside redeploy() and it allows for any changes in implementation as needed. It also mentions only top level artefacts (those that are tested by this test). All others, needed as dependency do not need to be mentioned, they are implicit and will be resolved.

Real solution is a bit more complex than this example, but original pseudo code I have shown does not show everything too. Main point is it goes directly to essence of scenario setup - it shows nicely what we want to use in test instead of obscuring it with details how it is achieved. There are, however, a few even more important things that are not shown:

• It was easy to achieve optimization - to undeploy only unwanted artefacts, deploy missing ones and redeploy those whose dependency changed, everything in correct order. All without touching beforeClass at all. This was possible only because it was not done explicitly, but hidden inside of redeploy().
• whole code together with optimization was heavily unit tested, so I could do changes confidently, while original duplicated test setup was tested only indirectly through tests it was meant to configure. (To be fair, there was one bug found a year later - optimization did not work effectively under all circumstances).
• Less than 1 month later after introducing it, our team leader realized we can use whole framework and existing artefact implementations in installer, we just need to provide slightly modified Deployer.
• When I returned back from my Round The World travelling I found the framework was moved from our project into shared space and it is used by other projects. Currently we are thinking we will use it for yet another unforeseen application, but it is too soon to be sure. But I think it should be possible just by providing another implementation of Deployer.

Conclusion

I have hinted in Solution that duplication of code usually means you are missing some abstraction. If you find it and make explicit you might save a lot of pain you had before, because information hiding allows you to introduce new approaches or enhancements cheaply.

With correct abstraction and well distributed responsibilities you might be surprised how your piece of code will get reused. In early 90s everybody hoped for OO languages being a solution for reusable code, but somehow it seemed OO could not deliver. IMHO it was always problem with badly assigned responsibilities (and misused inheritance). This claim could be slight simplification, but there is a lot of truth in it.

I wish I was there at the time it was decided it will be published to wider audience. I have some suspicion I have made not-so-good-decision-after-all with how Artefact handles equality. It is OK for my original task, but with wider usage it seems to be restrictive and confusing. And it is too late to change now after it is published contract ;-)

I am slowly reading blog of J.B. Rainsberger. I found it years ago thanks to book JUnit Recipes, but somehow I lost it.

I have found very interesting article there - The Four Elements of Simple Design. I find it fitting very well into my experience with design.

I claim that developing strong skills of detecting duplication, removing duplication, identifying naming problems, and fixing naming problems equates to learning everything ever written about object-oriented design.

Yes, developing skills and wanting to fix a problem every time it is spotted.

From time to time I am confronted with a piece of code that contains a lot of static methods in class (or several classes) and I struggle to use it effectively for my needs.

Only a few weeks ago I was using class from internal testing library and I needed to do something unforeseen by its author. It was utility class with many static methods. I have looked at its source finding about 1600 lines of code (and javadoc, so I do not know exact number of executable lines). All of them were quite long and doing real work instead of being a Facade and delegating elsewhere. There were also many private static methods and some of them seemed they could help me if I only could call them. Unfortunately I could not modify library so I tried to talk with its author and explain him that proper OO design would help a lot in accommodating unforeseen usages in the future. Hmm, I have learned from that discussion and follow-up I still need to learn how to sell my ideas in diplomatic way...

Then only a few days ago I was handed example code for solving similar problem I have been working at that time. It was rather well factored, responsibilities were clearly separated and everything looked fine. But I could not adapt it easily to my needs, because it contained many static methods. Even worse - static methods invoking static methods from different classes. So the only way I could use knowledge and time invested in that code was to copy&paste it into my namespace instead of linking to that component and plugging in different implementation of parts which were different. *1

When I was thinking about this blog post I remembered similar situation that happened several years ago and that probably caused my strong aversion for static methods. I had been made responsible for maintaining and enhancing piece of code that was completely static. Again, several classes with defined responsibilities consisting only from static methods coupled together in non-extensible way. And my task was to add new report to that: "Here is reporting component, add these new reports. How hard can it be?" Well it was hard. All existing reports were done by copy&pasting from previous report, because it was only way to do that. Fortunately I had enough time to rework it completely, leading to nice extensible solution consisting from considerable less number of lines.

Why are static methods bad?

Previous section hinted that static methods prevent future growth of code, because they are too rigid and strongly coupled to other parts (static, of course). That means it is not easy to form different path of execution later while still preserving also old one (to implement unforeseen change). So static methods effectively fight against one of the main reasons why object-oriented languages are powerful. The possibility to extend existing code without modifying it (Open Closed Principle).

To make it even worse, once you start making class with static methods, it eventually leads to adding more static methods (because they are called from existing static methods). So at the end of our effort we are left with lot of procedural code, that is:

• hard to extend.
• hard to read, because all data are passed as parameters, making intention of invocation less obvious.
• hard to apply when requirement to solve slightly different problem arises.
• updated: they prevent small refactorings, because you cannot easily create attribute to keep some state instead of passing all data as parameters. If you introduced such an attribute you would need to consider thread safety while static methods using only parameters and local variables are easy to reason about. Without refactoring code gets ugly soon.

All this in the name of convenience at the beginning of the process. "I want to make my classes to be convenient to use and the best way to achieve it is that clients do not need to instantiate 'unnecessary' class". Yes, it can be convenient in the beginning, but it usually evolves to inconvenient rather quickly. Let's see some examples:

public void configureComponent(Component configuredComponent) {
    ComponentUtil.setStrictValidation(configuredComponent, true);
    ComponentUtil.setCollaborators(configuredComponent, collabCollection);
    ComponentUtil.restart(configuredComponent);
}

Previous code shows usage of utility class with static methods. Notice that configuredComponent is passed into all methods. It has to be, because utility class does not own Component and it should not own it, otherwise it would not be thread safe (as you could not configure more components at once).

Now, let's see what would happen if we decided that configuration of component is important enough to deserve its own class with life-cycle (instance created).

public void configureComponent(Component configuredComponent) {
    ComponentConfigurator configurator = new ComponentConfigurator(configuredComponent);
    configurator.setStrictValidation(true);
    configurator.setCollaborators(collabCollection);
    configurator.restart();
}

As you can see "inconvenience" to create instance is quickly replaced with less typing needed in all method invocations. Additional advantages (probably not visible in this simple example) are:

• instance of ComponentConfigurator is configuring sole instance of component and it can use attributes to store intermediate state without a need to pass it to caller and expecting it to be passed back.
• it is easier to reason about code (what it does, what change is safe, what potential bugs can happen and so on) if it is self-contained and encapsulated.
• it is easier to unit test.

How to avoid them

I am not going to write how to get rid of existing static methods. Just refactor the code until they are replaced with normal methods.

Instead, I am going to explain how we can avoid them to happen. The best way how to avoid having many static methods in your code is not to write them in the first place. Any time you are creating new class and it looks like it does not need to have any state avoid temptation to make it static. Any time you think "I will put this code into separate utility class, because it has potential to be reused elsewhere" avoid temptation to make it easy to call. Contrary - make it harder to call by asking to create instance of that class. It gives you much more flexibility, because you can pass it to different parts of application, you can compose it with other objects and do all those nice object-oriented things. But static methods can only sit in its class and hope they will be called ;-) Following example might look as overkill to you now, but later when more responsibilities are added into that class you will be glad you decided to do it "hard way".

    new SomeUtility().performOperation(data);

I am not against static methods completely. For example Math.abs() is useful, but only because there is no better place to put it to as primitive types in Java cannot have methods. Later, as the utility class grows you might recognize its true purpose or to find some part of it can be identified as standalone concept in your domain and renamed accordingly (or moved into different class). And of course you might realize later that static method would be useful, because it does not need any attributes (question you should ask is if it really belongs to given class or not). So make it static then. It will be more convenient to call after that. But if you make decision about being static too soon you will end up with unmaintainable code sooner.

Note about class naming

Do not forget that class name suffixes Helper, Utility, Manager and Controller are by many developers considered problematic, because they are an excuse to collect a lot of functionality that should belong into different classes, leaving them to be pure data structures. It is another example how OO language can be used for procedural programming. Utility and Helper fit well into this article as they are also good candidates to consists from static methods!

---
*1 Terrible solution, but I had only a few days for finishing my task without having serious understanding of used technology and protocols, so leaning on existing stuff was only way.

My Dilbert 2.0 book has just arrived :-) Finally I can learn all missing tricks about OOP.

Oh, did you know that all I know about OOP I have learned from Dilbert? If not, you have probably missed this old, but very good article. It was pointed out to me by my experienced former colleague and it really made the difference back then.

Now it's pity that Dilbert 2.0 is meant as Xmas present to me from my wonderful fiancée, so I need to wait...

I found following sentences regarding quality in Art of Agile Development while commuting to work this morning.

The best way I know to reduce the cost of writing software is to improve the internal quality of its code and design. I've never seen high quality on a well-managed project fail to repay its investment. It always reduces the cost of development in the short term as well as in the long term. On a successful XP project, there's an amazing feeling - the feeling of being absolutely safe to change absolutely anything without worry.

I wish more people were aware of this and acted accordingly. I have to make sure this happens at least at my present and future jobs.

Recently I have logged into internet banking application of my bank in Slovakia. I found an ad offering me a loan. That is fine, I like to be informed about potentially useful things. This time, however, I was not interested. But I could continue only with two paths "Contact me, I am interested" and "Show me this offer later". This looked like unintentional omission of third possibility "Thank you, do not show me this again". So I contacted the bank and told them about this.

I've got reply next day (yes, they have good customer service). From the answer I have learned that that option was there before, but sometimes people clicked "I do not care" and then, some time later they wanted to apply. So the bank decided to solve this glitch. And they did. Problem is, that solution seems worse than original problem. People not interested in that offer in current version of internet banking are supposed to click "Contact me, I am interested", that will present them a form they need to fill out. The critical part is to leave the form empty. That handles missing state "Thank you, do not show me this again" with possibility of changing your mind later, because empty form will be available in "empty forms" folder.

Huh? How was I supposed to figure that out? And what about people that do not keep in their mind that you need to click Start if you want to shutdown Windows? I do not know what designers/programmers in my bank were thinking about, but I expect I am not the only person who would not click "Contact me" option if I do not want to be contacted. It seems to me that someone suggested the cheapest technical solution how to add new this new functionality - just remove existing feature and existing code will handle rest of it. I am sure they did not think about people using their product.

I hope it is only workaround until they implement, test and deploy working solution.