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.

"Best practices" are everywhere in our industry. A Google search on "coding best practices" turns up nearly 1.5 million results. The idea seems to bring comfort to many; just follow the instructions, and everything will turn out fine.

When I read about a best practice - for example, I just read through several in Clean Code recently - I get nervous. Does this mean that I should always use this practice? Are there conditions attached? Are there situations where it might not be a good practice? How can I know for sure until I've learned more about the problem?

Several of the practices mentioned in Clean Code did not sit right with me, but I'm honestly not sure if that's because they're potentially bad, or if that's just my personal bias talking. I do know that many prominent people in the tech industry seem to think that there are no best practices, so at least my nagging doubts place me in good company.

The number of best practices I've read about are simply too numerous to list here or ask individual questions about, so I would like to phrase this as a general question:

Which coding practices that are popularly labeled as "best practices" can be sub-optimal or even harmful under certain circumstances? What are those circumstances and why do they make the practice a poor one?

I would prefer to hear about specific examples and experiences.

share
8  
Which are the practices you disagree with?, just curious. –  Sergio Acosta Oct 28 '10 at 13:17
5  
@Walter: This has been open for months, it is definitely constructive, why close it? –  Orbling Jan 18 '11 at 2:23
2  
Seeing as how my name is being mentioned in here, I suppose I should chip in: I believe that the answers here are valuable, but the question could be reworded into something definitively less poll-like without invalidating any of the answers. Example title: "Which popular 'best practices' can sometimes be harmful, and when/why?" –  Aaronaught Jan 19 '11 at 1:33
show 11 more comments

locked by ChrisF Mar 15 '12 at 21:47

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

closed as not constructive by Mark Trapp Nov 20 '11 at 17:22

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

45 Answers

Absolutist, strict, encapsulation

I agree with encapsulation mostly, but it was a real liberator to temporarily break that rule in order to convert some procedural code to OOP using the Array to Object refactoring. (Needed a bit of a push to loosen up a bit. Thanks, Martin Fowler)

  • Array

    ->

  • new class with array as public property

    ->

  • go through client code / add accessors

    ->

  • only then encapsulate it and make it private/protected

share
show 1 more comment

Over generalization.

The square block does NOT fit into the circle hole!

share
6  
Since when was "over generalization" a best practice? –  Peter Alexander Oct 26 '10 at 20:59
8  
Actually, a square block can fit into a circle hole, it just has to be smaller, or you have to use a lot of force. –  Malfist Oct 27 '10 at 15:59
show 3 more comments

I tend to ignore almost all best practices when doing a relatively small, one-off project, like writing a parser required to migrate files. When all I need is the result, once, I don't care about readablitiy, scalability, maintainability, etc.

share
16  
Please God, do not let me work with you. For two reasons: First, I agree, to a small extent, but some practices not only improve code, but improve development time. Second, I have come to realize that there is virtually no such thing as "one off code". If it was useful to you, there is a very good chance it will be useful to someone else. –  Stargazer712 Nov 22 '10 at 4:44
4  
For those small one-off things, I don't have to work with anyone. I'm talking about programs with something like 100-200 lines of code, no teamwork required, thank you very much. IMO it's better to be honest with those things, declare them as disposable and do not make a big deal about them. Otherwise, you might be tempted to spend weeks on building the ultimate "data transformation and import framework" just to make sure that your program could hypothetically be useful for the yet unknown next task of that kind. –  user281377 Nov 22 '10 at 8:18
2  
I've done that and seriously had it come back to bite me. The best practice of 'Code Reuse' might breathe life into a shambling monstrosity you hacked together, and the next thing you know it's crushing downtown New York while you scream "YOU WERE NEVER MEANT TO BE USED!" at it from your jail cell, where you await execution for having created that abomination. –  Trevel Jan 17 '11 at 22:32
show 9 more comments

Only one I can think of is formatting Java if statements like this:

if (condition) ...

instead of

if(condition) ...

For the most part, I've found best practices to be best, and any initial reservations I have with them tend to be misplaced, or overridden by more significant concerns.

share
2  
I like if( condition ) { –  Carson Myers Dec 1 '10 at 3:49
7  
I don't agree .. keyword (logic) vs function(paramaters) just feels more natural to me, but style is a very individualistic thing. Same thing in C. –  Tim Post Dec 10 '10 at 8:02
add comment

"If it ain't broke, don't fix it". Code is always broken. The reason nobody is telling the original developer is that they

  1. think it's their own fault,
  2. don't know who to contact,
  3. can't get through support to speak with a "techie",
  4. can't be bothered, or
  5. don't know how to describe the problem in enough detail.
share
2  
You forgot: The original coder is dead. –  Christopher Mahan Jan 20 '11 at 22:16
3  
If the code is proven to match the requirements, most likely the requirements are broken. –  Tom Clarkson Jan 21 '11 at 8:37
show 1 more comment

Always favour long, explicit, variable and routine names over the shortened form

e.g favouring Authentication over AuthN

This bit me, when MS file paths got so long that my subversion working copy could not load onto my PC.

See: http://stackoverflow.com/questions/3282303/my-choice-of-class-names-is-hampered-by-windows-xp-max-path-length-issues-with-sv

share
2  
Usually, name lengths are shortened by combining similar methods into namespaces/classes. For instance, I'd put that function under CustomerNotifications.PaymentReceived(). That's more a limitation of file-based-MVC frameworks and/or poor design than the fault of variable naming conventions. And yes, I prefer Authorization over AuthN because, what is N supposed to represent anyway. To the person naming it, it's clear, to others it isn't. –  Evan Plaice Jan 18 '11 at 1:54
show 1 more comment

In JavaScript many people consider it best practice to write all the semicolons they would need in other C branch languages, (and a lot of them ain't afraid to point out the "error" whenever they see code that could have more semicolons). I never found a need for them, their role as visual marker is already filled by line shifts.

On a side note it is kinda funny to see a compiler requiring semicolons, but still confidently throw the error "Missing semicolon". "We both know what you meant to write, but I must reject your application because I formally ain't allowed to distinct between spaces and line shifts."

A number of other JavaScript/C-style best practices are circulating, but none of them come close in terms of hype without qualified arguments.

Edit: It is perfectly possible to minify code without semicolons Closure Compiler will insert them as needed.

share
2  
Douglas Crockford says to put the semi-colon, so I put the semi-colon. –  PeterAllenWebb Nov 30 '10 at 1:38
3  
The purpose for this is so that minifiers don't screw it all up. The line breaks only work while they still exist. –  Paul Butcher Nov 30 '10 at 11:18
11  
I can't stand people saying, "Well the compiler complained about the semicolon, why can't it just put one there!?" The answer is because you did not put one there. Anyone who worked with Word 2003 enough knows that computers are terrible at guessing intention. Compilers are even worse. The "semicolon expected" error just means that based on the crap you typed, it looks like a semicolon might work here. It could be that you were missing a semicolon, or it could be that you typed an intelligible line of crap that cannot be read. It is not the compilers job to read your mind. –  Stargazer712 Nov 30 '10 at 15:19
show 4 more comments

Databinding

I hate it and love it. (Depends on complexity of the project)

share
3  
I'm in the same boat. I hate it and love it at the same time. Can you detail what you hate and love about it? –  Steve Evers Dec 10 '10 at 16:06
add comment

Threading is the answer for all performance problems

Disagree with that for many reasons.

  • In general indexing and temporal caching gives much greater boost in performance. Of course you might actually need to use "dirty flags".. Making your design all messy and stuff..

  • Problems that come from multi-threading overweight all benefits time and money wise.

  • What multi-core-technology really brought us is better multi-tasking (running couple of applications just as fast as one). Utilization of skillfully written multithreading programs is just a side effect.

share
show 3 more comments

Well, one of my hot buttons is performance analysis. People say

Premature optimization is the root of all evil.

And then they say the complete quote is:

We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%.

Which makes perfect sense when you're discussing "algorithms" - clever academic stuff where 3% of the program might be two lines of code.

I work in 10^6 line apps, with 10^5 methods in 10^4 classes, worked on over years by a dozen programmers and testers, undergoing numerous revisions and releases, where that quote has no relevance at all. Typically the time drains are single lines of code which may be "opportunities" but no one could ever guess where they are.

OTOH, people say "profile", or they say "measure measure". Well, fine. They say it, but they don't often DO it. Could be, the reason they don't do it is it doesn't work very well.

This answer says more about that, and points out what does work.

share
add comment

CONTINUE STATEMENT IS BAD

I'm going to reference the Joint Strike Fighter Air Vehicle C++ Coding Standards here:

AV Rule 190 (MISRA Rule 57) The continue statement shall not be used.

and a professor of mine:

"THE CONTINUE STATEMENT IS BAD"

Their logic is that it disrupts control flow and makes the code less readable. Using nested if statements is a cleaner approach.

Yet, I find this more readable:

 
for(int i = 0; i < CONST_FOO; ++i) {
    if(aTwo[i] == NULL) continue;
    aTwo[i]->DoBar();
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
    //...do other stuff with aTwo[i]
}
 

Than this:

 
for(int i = 0; i < CONST_FOO; ++i) {
    if(aTwo[i] != NULL) {
        aTwo[i]->DoBar();
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
        //...do other stuff with aTwo[i]
    }
}
 

This is a very simplistic example, (imagine the nested ifs go a lot more than one level) but it is similar to the ridiculous "only one return statement per function" "best" practice. It prevents the debugger from jumping around (possibly several pages away), less indentation generally makes easily readable code, etc.

share
add comment

Using static and final keywords.

static, because its faster.

final, because you dont want anybody to extend that class(aka Design For Extension).

it makes class using those classes very hard to unit-test.

share
3  
As a best practice, classes are marked final in frameworks because it is hard to predict how a user might extend them. Consequently it is safer to seal the class than it is to try and figure out every possible way your users might abuse the class. See blogs.msdn.com/b/ericlippert/archive/2004/01/22/61803.aspx –  Robert Harvey Oct 27 '10 at 22:05
4  
Also, I'm not a big fan of testing frameworks that require you to inherit from the classes you are testing. There are plenty of test frameworks out there that don't require this, making it straightforward to test sealed classes. –  Robert Harvey Oct 27 '10 at 22:08
6  
@Robert Harvey: It's safer to seal the class - safer to whom? If someone extends a class, isn't he responsible for figuring out how to do it properly? I think sealing the class just for the sake of dogma is like obfuscating your source code before putting it into version control. Yeah, safe, kind of, because then nobody else can change it. That kind of safety just doesn't come for free. –  Joonas Pulakka Oct 28 '10 at 6:53
1  
Granted, this depends on the context. I definitely understand your stance if you're making a software library product for customers who code on top of your code; you're then also protecting yourself from having to freeze some implementation details because your clients happen to depend on those. It's quite different from making code within the house where just a few programmers ever interact directly with your code (in which case keeping the code "open" is quite an useful general practice). –  Joonas Pulakka Oct 28 '10 at 15:05
show 6 more comments

Static typing of primitives

Having to specify a data type as one of a large number of types of number is premature optimization. Just call it a number, keep track of its value, and assign a type later if it's useful for optimization.

User-defined types are useful. Inherent types aren't.

share
3  
The behavior of each of the numeric types differs significantly. For example, the decimal type in c# is suitable for financial calculations, while the 'float and double types are not. int will handle negative numbers; uint will not. You might as well choose a suitable type to start with. –  Robert Harvey Oct 28 '10 at 20:54
4  
I submit that if you don't already have a pretty good idea of what the variable is going to be used for, you don't need it (yet). See also docs.sun.com/source/806-3568/ncg_goldberg.html –  Robert Harvey Oct 28 '10 at 21:06
1  
@Robert: And I believe that, if you want a number, you shouldn't have to worry immediately about expected size and signedness. I found it ironic that you included a link for what everybody needs to know about floating-point numbers: it should be possible to program without keeping that entire paper in mind (much as I like that particular paper). –  David Thornley Oct 28 '10 at 21:15
3  
If I need a general number, and it doesn't have a decimal point in it, I generally use an int. That covers about 95% of all cases where I need a number. –  Robert Harvey Oct 28 '10 at 21:42
6  
@Codex: Ah, no. I don't want the compiler making that decision for me. Changing types has all sorts of profound implications on the behavior of the code (casting and type conversion, for example). The compiler is not smart enough to figure out my intentions, nor should it be. I specify my intentions by telling the compiler what numeric type I want it to use. –  Robert Harvey Nov 1 '10 at 16:19
show 4 more comments

I don't see anything wrong with:

if ($variable = some_function_call()) {
  do_some_stuff($variable); # etc.
}

but some people (clearly Quiche Eaters) insist that the single equals sign is easily mistaken for an ==, which I've never had a problem with—and neither have any of the programmers I've ever worked with.

share
2  
Is this assigning the result of some_function_call() to $variable, and then using $variable as the boolean result for the if? Yuck. This adds too much cleverness for not enough benefit, and makes it difficult to figure out what your intent is. Do the world (or at least the programmer coming after you) a favor, and separate it into two operations; the assignment, and then the if check. –  Robert Harvey Jan 12 '11 at 18:23
show 5 more comments

Using const-keyword in c++

How hard is it to understand that functions starting with "Get..." are normally "const". Also has anyone seen evidence that compiler optimizes const-riddled code better? At least when comparing the following two later one performs faster (when optimization is enabled):

__forceinline void test(const int a);
test(1);
test(2);
test(3);


template<int a>
void test();
test<1>();
test<2>();
test<3>();
share
5  
Have to disagree here; there are cases where const is required, mainly for operator overloading to work properly. (This probably why you are getting down votes.) –  Jay Jan 12 '11 at 18:31
show 4 more comments

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