Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a codebase where the programmer tended to wrap things up in areas that don't make sense. For example, given an Error log we have you can log via

ErrorLog.Log(ex, "friendly message");

He added various other means to accomplish the exact same task. E.G.

SomeClass.Log(ex, "friendly message");

Which simply turns around and calls the first method. This adds levels of complexity with no added benefit. Is there an anti-pattern to describe this?

share|improve this question
35  
"Bad coding" covers it ;) –  Oded Nov 14 '12 at 14:36
11  
Depends. Is it a wrapper for a logging library? If there ever was the chance it could be swapped this actually is a good practice... –  Rig Nov 14 '12 at 14:50
2  
Maybe baklava code? –  lortabac Nov 14 '12 at 15:37
3  
@lortabac: The problem with "baklava code" is that it acually sounds good and tasty, and therefore desirable. –  FrustratedWithFormsDesigner Nov 14 '12 at 15:38
9  
What does this programmer say when you add them why are they adding these methods? Understanding their reasoning should make it easier to educate them. –  Keith Nov 14 '12 at 17:19
show 9 more comments

13 Answers

up vote 54 down vote accepted

It is only worthwhile to call some bad coding habit an Antipattern if it is reasonably widespread.

The rest we just call "rubbish code" ...


If I was to suggest a name for this particular bad habit, it would be "Obsessive Abstraction Disorder" :-)

share|improve this answer
9  
I've always used "Indirection Hell". –  Dan Neely Nov 14 '12 at 20:18
add comment

No, it's not a anti pattern but I would raise the following issues.

Violation of Single Responsibility Princple

SRP says that a class should have one reason to change. Adding a logger method means that the class also have to change if the logging logic should be changed.

Violation of a is-a relationship

Base classes are not toolboxes where you can add several convenience methods.

Doing so effectively couples all inherited classes to the implementations that the base class has.

Compare that with composition.

share|improve this answer
1  
"Single Responsibility issue"? SRP? Perhaps you meant to write Single Responsibility Principle? Just connecting the two here. :) –  zxcdw Nov 14 '12 at 23:05
    
@zxcdw: yeah. thanks –  jgauffin Nov 15 '12 at 6:00
add comment

Not that this makes it acceptable but this could just be an unfinished refactoring. Maybe the SomeClass.log() had it's own logic an was implemented first. And later they realized that they should be using ErrorLog.Log(). So rather than change 100 places where SomeClass.Log() is called, they just delegated to the ErrorLog.Log(). Personally I would change all the references to ErrorLog.Log() or at least comment SomeClass.log() to say why it delegates the way it does.

Just something to consider.

share|improve this answer
add comment

The term "Lasagna Code" comes to my mind, though apparently it means different things to different people. My interpretation has always been "layered for the sake of being layered".

share|improve this answer
2  
I've also heard "Onion Code" to describe the same thing. –  Justin Niessner Nov 14 '12 at 19:46
add comment

I'm not sure to describe this as an offense to the single responsibility principle, because if there is a change to the ErrorLog method signature (the method that is called by SomeClass), every client code that calls this method will fail.

There is obviously a way to use inheritance (or making classes that needs logging implementing a logging interface) there.

share|improve this answer
    
Still bad practice because: 1) it's unlikely to change 2) if it does change, they can just build a wrapper class with the same name and change the imports. –  kevin cline Nov 14 '12 at 16:58
2  
+1. And here's "Uncle Bob" (Robert Martin) arguing in favour of centralising dependencies in a limited number of modules, rather than scattering them through the code. –  MarkJ Nov 15 '12 at 15:56
add comment

Or we could look at this as a "teachable moment" :)

Your developer may be halfway to a good idea. Suppose you want to have the requirement that all of your business objects are capable of intelligent logging. You might define:

   public interface IBusinessObjectLogger
   {
       void Log(Exception ex, string logMessage)
   }

Now internal to your objects you can use the ErrorLog object to actually do the logging while SomeClass code adds object specific value to the log message. You can then further extend your logging APIs to implement file based, db based or message based logging functionality without touching you business objects.

share|improve this answer
add comment

I'm not sure this is automatically evil.

If you're calling SomeClass.Log it most certainly is evil but if Log is only being used from WITHIN SomeClass it's cutting down the coupling and I would call it acceptable.

share|improve this answer
add comment

This is actually pretty common in certain coding styles, and the basics of the line of thought are not, in themselves, an anti-pattern.

It's very probably the result of someone coding by necessity without knowledge of the wider codebase: "OK, this code needs to log an error, but that function is not the code's primary purpose. Therefore, I need a method/class that will do that for me". That's a good way of thinking; but, without knowing ErrorLog existed, they created SomeClass. They then found ErrorLog at some later point, and rather than replace all usages of the method they'd put in, they made their method call ErrorLog. That is where this becomes a problem.

share|improve this answer
add comment

Accidental complexity is complexity that arises in computer programs or their development process which is non-essential to the problem to be solved. While essential complexity is inherent and unavoidable, accidental complexity is caused by the approach chosen to solve the problem.

http://en.wikipedia.org/wiki/Accidental_complexity

share|improve this answer
add comment

It may be an abuse/misunderstanding of the Facade Pattern. I have seen similar things in a codebase I've worked with. The developers went through a phase when they were crazy about design patterns without understanding the overarching principles of OO design.

A misuse of the Facade pattern results in a blurring of the levels of abstraction across the layers of the application.

share|improve this answer
add comment

What this programmer is doing is wrapping some code, so that it does not have a direct dependency on the logging module. Without more specific information, it's not possible to give a more specific pattern. (That these wrappers are not useful is your opinion which is impossible to agree or disagree with without much more information.)

Patterns that this may fall under are Proxy, Delegate, Decorator, Composite or some such which have to do with wrapping, hiding or distributing calls. It may one of such things in an incomplete state of development.

share|improve this answer
add comment

It's a violation of the abstraction principle. There are a lot of resources on the web and literature about abstraction principle but the Wikipedia article is a good start.

Abstraction principle is like having single copy of stuff. It's good because repeating yourself is bad. If you repeat yourself you are more likely to create errors!

Abstraction is good. So when we create copies of stuff, they start to become redundant, and look bad too. So it's not OK to create unnecessary copies. We should avoid that.

When we violate that principle that is actually called "violation of abstraction principle".

And here is a quote from its Wikipedia article:

The effort of rewriting a piece of code generically needs to be amortized against the estimated future benefits of an abstraction.

share|improve this answer
1  
You went a little over the top and we didn't say you couldn't link to external sources, just that they shouldn't be the entirety of the answer. –  ChrisF Nov 15 '12 at 15:39
add comment

I could imagine it to be cultural difference. Maybe there is a good reason for having duplicate functionality in this particular code base. I could image ease of writing to be such a reason. The Python programmer in me would still say it is bad, since "There should be one-- and preferably only one --obvious way to do it.", but some Perl guys might be accustomed to the fact that "There's more than one way to do it".

share|improve this answer
1  
Ease of initial writing is not a good reason for duplication. Bad code might be easier to write the first time around, but that does not make it easier to write code in the long-run. What does the new guy coming in do with the duplicated functionality? Which method does he call? He wastes time looking them up and reading them, only to find they do the same thing. –  Kazark Nov 15 '12 at 0:12
    
Maybe this code was originally meant as a prototype that then got (ab)used as an increment. In that case, optimizing for initial writing would have been valid, I think. –  Bengt Nov 15 '12 at 17:41
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.