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'm working on a legacy system that has a helper class that is symbolically linked into many different .Net projects within a solution. The logic is riddled with compiler directives that change it's internal behavior based on which .Net project it happens to be compiled in. For instance, there's a compiler directive that interogates a "HAS_UI" variable and if true, compiles in a frmProgress instances and displays progress to the user.

I'm trying to explain to colleagues why this is a poor design by referencing specific anti-patterns. All I can come up with so far are "positive" patterns that should have been used, like the Single Responsibility Pattern or the Observer pattern.

Can anyone suggest specific anti-patterns that might apply here?

Thx

share|improve this question
2  
What you're asking is akin to asking whether using if statements is bad practice because you've encountered poorly-written code that uses them. Resorting to ticking off boxes on a list of "anti-patterns" says you haven't studied the code well enough to understand and articulate the problems on your own. –  Blrfl May 11 '13 at 11:07
    
@Blrfl, I'm not sure how you've been able to come to the conclusion that I haven't studied the code well enough. In fact, I've studied this code for months. All I am asking is for some assistance in naming a few more anti-patterns to see if they're relevant and can help me to better draw parallels to other examples. Does that make sense? –  sisdog May 13 '13 at 3:56
    
I just think you're going about this the wrong way. Telling people who don't already understand that these things are wrong or what they're called that "this code contains anti-patterns X and Y" will leave them no better off than they were before. You have bad code at hand; take advantage of it by explaining in very specific terms why and how it's going to bite them in the future and what can be done do to prevent it. Showing that you understand it and that you can articulate them to an audience that doesn't will give you a lot of credibility and a lot of leverage in improving things. –  Blrfl May 13 '13 at 12:37

4 Answers 4

"It depends"

Compiler directives can be exceptionally useful tools. They can provide ironclad rules that keep development (debug) acceptable aspects out of production (release) builds. Likewise, an assembly can be made more lightweight by removing tracing instructions once the build is switched to release.

On the other hand, they can be abused. I once had the displeasure of pulling a shoddy API apart that had directives for 7 or 8 different platforms as well as debug & optimized code. It was a bug-ridden mess that wouldn't compile in my environment.

Beware of falling into your own anti-pattern of "I didn't write it so therefore it's crap." Presumably, the project has had at least one release and has held up under a degree of maintenance. It's an anti-pattern to declare the code as crap simply because you can't wrap your head around it.

To be clear, I'm not saying that what your project is doing is right but it's not necessarily wrong. The impetus is upon you to fully understand what's going on within the code and then be able to explain why it's wrong. The examples you provided didn't jump out and scream "ew, that's bad" so I suspect you may have more research to do with the code.

Ultimately, you should be able to point at specific sections of code and say "this is bad because of XYZ." You shouldn't need to rely upon colloquial anti-patterns to prove your case.

share|improve this answer
4  
+1 - "It's an anti-pattern to declare the code as crap simply because you can't wrap your head around it." ... ::<<Loud applause>>::. –  Stephen C May 11 '13 at 0:32
    
@StephenC - dirty secret: it's taken me a while to recognize that behavior in myself... :-( Thanks for the comment! –  GlenH7 May 11 '13 at 0:52
1  
Some excellent points here. Things you never hear in a software development office #390: "Why yes, I'd have written that code exactly the same way!" –  Robbie Dee May 11 '13 at 10:25
2  
@RobbieDee: Once in awhile you do hear its close cousin, "Hey, that was very nicely done." –  Blrfl May 11 '13 at 11:16
    
@GlenH7, thx for the ideas. I've spent a ton of time in this code. I'm not looking for anti-patterns to justify "my case" because my goal is not to indict anyone. And I'm not declaring this code as crap just because I didn't write it either. All I was asking for was some help identifying patterns that might help to describe the situation and draw parallels with others that have found themselves in the same boat. –  sisdog May 13 '13 at 5:20

I think you're taking this from a backwards approach. Anti-patterns are a way of naming things that are known to have bad characteristics - they are diseases, not symptoms. It's really the symptoms we care about, even if we haven't named this specific disease yet.

The way you should be talking about this is in terms of actual functional defects or deficits that this way of programming has:

  • It's nearly impossible to test in a meaningful way because the code depends on the environment
  • It's very dangerous to make edits because you have to test your change under so many different conditions
  • It's hard to read the code because you have to know the value of many external factors
  • The combinatorial complexity of all these flags makes a large number of configurations that have to be reasoned about
  • You have to recompile a large amount of code to see if you broke anything

And so on.

share|improve this answer
    
Hey thx @Ryan. From your description, if anti-patterns have bad characteristics then wouldn't the practice I'm seeing here (which I was hoping to name) have bad characteristics and therefore be worthy of being crowned an antipattern? I think the "Tightly Coupled Integration" antipattern is pretty close to my situation but I was looking for something that specific addressed overly aggresive code reuse through compiler directives the coupling of concerns that this is causing. –  sisdog May 10 '13 at 23:21
    
Compiler directives are useful and often necessary. The problem in your case is not the directives, but their improper use. I would say the antipattern is not the directives, but their use, and the project managment that led to that solution. –  isturdy May 11 '13 at 3:15

The logic is riddled with compiler directives that change it's internal behavior based on which .Net project it happens to be compiled in.

Compiler directives aren't the problem, they're just the means used to create the alleged problem. You'd probably have the same objection if you had to comment out and uncomment various blocks of code based on what kind of target you're building. At least the compiler directives let you avoid that.

So, the class in question relies heavily on conditional compilation. The main reason that that can become a problem is that it makes the code less clear -- you've got one piece of code that's trying to be eight different things at the same time. One way you can reduce the problem is to create target-specific subclasses of the class in question, and use compiler directives just to select the right subclass when you want to instantiate the class. That'll make the code easier to understand, but it can be difficult if the various targets each use different combinations of features.

share|improve this answer

I'm not sure it is an anti-pattern as such but it would seem to violate the SRP.

But that being said, there were probably perfectly good reasons the developer did what they did at the time.

Despite the great strides made in design patterns and various methodologies, software entropy will continue to be an unfortunate feature of legacy systems.

I'd be more worried about how you'd test such a thing. You want to make a tiny change in such code and you immediately don brown trousers as there are myriad branches of code that it could affect.

share|improve this answer

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.