Personal opinion on software development in general, development methodologies, tools and best practices in particular

Archive for April, 2010

Programming antipatterns

Did you ever do a code review where you recorded an extremely high amount of WTF/m? And did you ever wonder what the cause of all this bad code is? Most of the time cause number 1 are the use of design and coding antipatterns.

If you like definitions, here is one: An AntiPattern is a literary form that describes a commonly occurring solution to a problem that generates decidedly negative consequences. The AntiPattern may be the result of a manager or developer not knowing any better, not having sufficient knowledge or experience in solving a particular type of problem, or having applied a perfectly good pattern in the wrong context.

Reinventing the wheel
IMO an antipattern of frequent occurrence is the lack of knowing the existence of some useful frameworks/libraries. Apache commons lang and commons collections are dependencies that should be present in every Java project.
You can write your own fancy for loop for filtering or selecting some objects in a collection or you can use…) or CollectionUtils.filter(…).
You can do some fancy not null checks ending up with a huge if-else construction or you can use StringUtils.isNotBlank(…). It’s up to you.

A general rule is not trying to reinvent the wheel. Some people like to write their own Reflection utils while Apache commons beanutils, Spring’s BeanUtils and BeanWrapper will do the trick.

Cargo cult programming
Cargo cult programming is a style of programming in which patterns and methods are used without understanding why. The term “cargo cult”, originally referred to aboriginal religions which grew up in the South Pacific after World War II. The practices of these groups centered on building elaborate mock-ups of airplanes and military landing strips in the hope of summoning the god-like airplanes that had brought marvelous cargo during the war.

Most of the time this way of programming is used by unskilled or unexperienced programmers that copy-paste some code from somewhere else.

– adding unnecessary comments to self-explanatory code
– adding deletion code for objects that garbage would have collected automatically with no problem
– creating factories to build simple objects

Coding by exception/Expection handling
Instead of checking for some specific corner case values like null values, some people like to catch a NullPointerException and do some logic in the catch block. This way of coding is called expection handling because the exception is expected to happen.

Exceptions are invented to inform you of the fact that something really bad happened but they’re not meant to be thrown often. That’s why they are called ‘exceptions’. If they occur, please handle them carefully but never abuse them to execute some logic that could have been implemented with a simple if-else check.

Avoiding/swallowing exceptions
Referring to the previous antipattern, when an exception is thrown that means that something unexpected happened. The last thing you should do is swallow these exceptions instead of processing its useful information.

For example, if you have a method that should only return 1 object because you expect it be unique, perfoming a DB query that returns a list of results, checking if the list’s size equals 1 and then performing a query that can only return 1 unique result (if not an exception is thrown). If you’re implementing something like that, it means that it could be possible that the expected object is not unique which means that implementation differs from what the analysis says.

Inheritance hell
Inheritance should be handled with care. It’s very useful but you should only use it what it’s intended for. If the inheritance tree becomes to bloated, something is wrong. Do not write abstract classes for 1 specific case. Use composition instead. The strategy pattern can come in handy here.

For example if your JSF managed bean ‘EditUserManagedBean’ extends AbstractEditingManagedBean which extends AbstractSelectionManagedBean which extends AsbtractParentDetailManagedBean which extends AbstractManagedBean you should know that something is wrong and that there should be other ways to implement this behaviour 😉

Gold plating & Premature optimization
Some people like to enhance their code by continuing to work on it well past the point where the extra effort is adding some value instead of sticking to the requirements. The mistake made here is thinking the end user would be much more delighted to see the additional or enhanced features in the product, than what the user asked for or expected. The user might be disappointed in the results, and the extra effort by the developer might be futile. This process is referred to as gold plating.

Gold plating is related to premature optimization. Premature optimization refers to thinking about possible issues that could arise in the future but that are not the case at this moment. You should only think about what is asked and what is required, not about what eventually could be useful for future purposes. A frequent occuring premature optimization is the premature performance optimization. If there are no preformance issues at this moment, do not try to handle them, handle them when they happen.