Best Practices Gone Bad

G. Wade Johnson

Campbell Johnson

What Are Best Practices?

Commercial or professional procedures that are accepted or prescribed as being correct or most effective.

Practices agreed on by a given community.

Best practices serve as a kind of mental shorthand to avoid having to rethink how to do everything each time. This also helps less senior people do mostly the right thing, even if they don't understand why.

How Can Best Practices Go Bad?

They don't.

People misuse these practices.

Best practices do not cover all of the reality of a particular process They mostly work as a reminder if you already know what you are doing. They also serve as a good starting point for those who are completely lost.

Why Do People Misapply Best Practices?

This is easier to explain after some examples.

Honestly, you wouldn't believe me if I told you now.

A Tale of Two Lengths

The goal is to have readable, maintainable code.

Define coding standards to enforce consistency.

This is an example from a place I worked years ago. They had a very rigid set of coding standards/coding style. I was not able to get a decent explanation of most of the rules during the short time I was there.

The Practices

The first two points were specified in the company standards. There were no provisions for violating the rules under any circumstances. The final is a common coding practice in the Java community. Since the code was written in Java, it was not surprising that we saw it.

Results of Tale


    SpecialResultClass resultOfInterestingAction
        = businessObjectCollection.getItemByIndex(
            theIndexOfTheObjectOfInterest
          ).performActionReturningInternalObject()
           .performInterestingAction();

Obviously, the names have been changed to protect the guilty. This is actually pretty close to some of the code I saw while working there.

Take a moment to see if you can tell what is actually happening.

Clearer, Less-Cluttered Approach


    Result result = businessObjs.get( i ).getFoo().act();

Except for a little license for too generic names, this is much easier to understand. Even if the variable and method names are shorter, it is still as clear as the original (if not more so). And, as a bonus, it doesn't take up 20-25% of a screen.

Why did it fail?

The two best practices were pushing in opposite directions.

These results clashed especially with language common practice.

This is a case of two best practices chosen with reasonable thought individually, but with no thought to how they would interact. To make matters worse, when it was obvious the rules were conflicting they just got more stubborn about enforcing the rule.

Unnecessarily DRY

The DRY Principle


Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Pragmatic Programmer, Hunt and Thomas

I originally saw this as the One Right Place principle from P. J. Plauger's book Programming on Purpose. Each piece of knowledge in a system should be be represented in only one place. And this place should be the obvious, or right place for that information.

What most people hear

Remove duplication.

Despite a very clear explanation of what the authors meant by DRY, what most people hear is don't duplicate your code.

Overly DRY Constants


   constant FOUR => 4;

   $hour = DECEMBER;

   my @months = ('') x DECEMBER;

In the first case, we probably had a series of uses of the literal 4 with no obvious relation. So, the programmer gave a very generic (and therefore, useless) name for the constant. In one system I maintained a long time ago. The constant FOUR had actually been given the value 5 at some point.

In the other two cases, we have a constant for the month of December and the code uses it inappropriately. What's worse, depending on how the value is used, it's obvious value could be either 11 or 12 and we have no idea which.

Overly DRY Code


   sub process_items {
       my ($which, @items) = @_;
       foreach my $item (@items) {
           if( $which == 0 ) { $item = ''; }
           elsif( $which == 1 ) { $item .= reverse $item; }
           elsif( $which == 2 ) { $item *= 2; }
           else { $item *= $item; }
       }
       return @items;
   }

In this case, someone noticed a bunch of loops that processed an array. To avoid having the loop in multiple places, they decided to copy them all to this one routine and provide a control variable to tell which thing to do. The first clue that this is a bad idea is the name of the function. It does mostly say what the routine does, but it doesn't tell you anything. Also, we will be doing one or more comparisons on the variable for each item of the loop. That's obviously an unnecessary waste of time.

Why did it fail?

The focus is on code/text duplication rather than knowledge duplication.

In most cases of DRY failure, the problem was focusing on the actual text that is duplicated and not on the knowledge. In both the constant and code cases, it's pretty obvious that focusing on the meaning rather than the code would have made us realize that even though the text is the same, there is no duplication.

Design Patterns

The Design Patterns book by the Gang of Four came out in 1995. The book made a rather sizable impression on people doing OOP at the time. Most people realized that this was an important piece of work. But not everyone agreed why. Interestingly, the Java programming language was taking off about the same time, and Design Patterns became very much tied up in what many Java programmers saw as the right way to write code.

Pattern Mania

The first comment usually comes from people who have never programmed in a time without Design Patterns. It sometimes also comes from people who had a religious conversion upon reading the book.

The second item is a joke based on a problem you see from junior programmers that first find patterns. After all, they reason, if one pattern is good using all of them must be better.

Although the Decorator approach to I/O that Java uses is quite elegant and allows a fair amount of flexibility without too many classes, they do not give a shortcut for the most common use of file input. This requires that you always have to create a FileInputStream and wrap it in a BufferedInputStream. Therefore, people who aren't aware of this are always stuck with unbuffered output.

The final is a joke about some Java programmers who use designs as the default way to solve any problem. It's not really a comment on the language. It's more about how it's often used.

Why did it fail?

Instead of using Design Patterns descriptively or as an aid to think about a problem. They are spread on the problem like a condiment.

If you really read the Gang of Four book, you can see that each pattern has not only suggestions on how it should be used, but also consequences of using the pattern. Most of the pattern zealots don't notice there is any downside. As with any form of abstraction, the code is somewhat harder to understand with a pattern in place. Unless the pattern really provides enough benefit, a simpler, less powerful solution may be better.

How to Do Design

Any reasonable sized programming problem requires some design.

This one is a little less clear than the others, because there is not one clear best practice for design. It's actually a complicated enough topic that people may easily disagree. Let's start off with a couple of approaches that most would agree don't work.

Code and Fix

You guys start coding and I'll go find out what they want us to build.

Many inexperienced programmers start solving problems with code before they have the slightest idea where they are going. The result is usually an unmaintainable, big ball of mud.

Big Design Up Front

To fix the problem with programmers coding without a plan, some decided that we should lay out the whole plan ahead of time so we know where we are going and when we will get there.

Excessive Flexibility

In trying to deal with the fact that things would change, many decided that we could design flexibility into the system. Portions of the design would be easy to change to prepare for things we guessed wrong or changes in the requirements.

This can work reasonably well if you are building a project just like ones you've built in the past. One might wonder why you just don't use one of them.

Push-back Against Big Design: YAGNI

Always implement things when you actually need them, never when you just foresee that you need them.

After dealing with too many flexible systems that were unmaintainable because the requirements never flexed where the code did, there was push-back to try to get back to simpler design again.

What is YAGNI?

Done correctly, this should reduce the code bloat and feature creep associated with trying to design for every eventuality.

YAGNI Overload

Of course, there's no principle in existence that can't be taken to extremes.

Why Did These Fail?

Too much up front design fails because we can't predict the future.

Too much YAGNI looks like head in the sand code and fix

Not surprisingly, the reality is a need to think when you are doing design. We need to think about what might be in the future, but code for the present. Resist the urge to code for things that might happen, but actually build anything that is required.

Why Do People Misapply Best Practices?

Unfortunately, two of the things that best practices should help us with are the same things that cause the best practices to fail.

  • Inexperience
  • Lack of time/inclination to think deeply about the problem

Questions?

References

Extras

Here are a few other best practices that I can think of that I didn't have time to cover.