Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

"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

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.

8  
Which are the practices you disagree with?, just curious. – Sergio Acosta Oct 28 '10 at 13:17
    
Anyone can write a book, and I do not have to agree with it - it is as simple as that. – Job Jan 12 '11 at 18:49
    
One thing I like about the book "Code Complete" by Steve McConnell is that he backs up all his tips with hard evidence and research. Just sayin' – JW01 Jan 17 '11 at 14:44
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

45 Answers 45

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
    
I agree, while using accessors is a good idea most of the time, it isn't always. It depends on how it's used. If the codebase is going to be used as a publicly accessible API, everything that's marked public should be encapsulated. If it's a library public and internal properties should be encapsulated. There are instances where it may make sense to break the rules but only if the internal implementation doesn't need to change later. For apps that have no API, I don't really see encapsulation as being necessary at all (and I usually do it anyway out of habit). – Evan Plaice Jan 18 '11 at 2:01

Over generalization.

The square block does NOT fit into the circle hole!

share
    
Agreed, but can you give an example? When you are building a framework, the complexity of generalizing things pays back many times over when you can use your widgets in flexible scenarios. – Robert Harvey Oct 26 '10 at 20:08
6  
Since when was "over generalization" a best practice? – Peter Alexander Oct 26 '10 at 20:59
    
Seems more like an antipattern I guess. – PMV Oct 27 '10 at 3:07
    
I would edit this to read "making things as general as possible". You've got to get into the land of concrete things at some point. – Jon Purdy Oct 27 '10 at 14:32
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

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
    
@ammoQ, that's what I'm talking about. 100-200 lines of code that you write alone. I once was on a project where I had to copy new files (and only new files) from server A to server B several times per week. I wrote a one-off script to do it. When I got moved to a different project, the new guy asked me what I used to do it, so I gave him the script. From there, the script was expanded to give daily reports of changes and has become a solid part of the development environment. There is no such thing as one off code. – Stargazer712 Nov 29 '10 at 22:32
    
@Stargazer712, best practices improve development time? This seems unlikely for the kinds of tasks (I believe) ammoQ is talking about. I recently wrote about 20 lines of python to patch together separate boot loader, kernel and rootfs binaries into a single image file. Should I have written a 'user story' about it, or played a few rounds of 'planning poker' to estimate it? Seems like overkill... – evadeflow Nov 29 '10 at 23:05
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

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

"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
    
Code can be provably demonstrated to meet all specified requirements, so I'm not sure what could be gained by declaring that "all code is broken." – Robert Harvey Jan 17 '11 at 19:49
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

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
    
I try to be explicit as much as possible. But, if you have to modify your whole design to accommodate the "best practice of favouring longer names" then something is wrong. I don't think 'Good design' ispo facto leads to shorter file paths. – JW01 Jan 21 '11 at 17:00

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
    
@Peter that's a terrible reason, unless what you really mean is "Douglas Crockford says the semicolon is a good idea because [...] and I agree with him." My personal reason is something like, "Douglas Crockford says to use the semicolon, and I don't remember why exactly, but I remember it made sense when I read it, so I've tried to adopt the habit." – MatrixFrog Nov 30 '10 at 8:29
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
    
Closure Compiler handles line breaks perfectly well, inserting semicolons as needed. Sure I have read that omitting semicolons makes code error prone, but I have never found any hard arguments, and my experience confirms none of the supposed problems. @PeterAllenWebb: You also remember to break all lines longer than 80 characters, as Crockford say you must? – aaaaaaaaaaaa Nov 30 '10 at 15:45

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

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
    
Hmmm... I agree with you; but I have to say that this "best practice" hasn't reached my dark corner of the Earth. Through the years, many teammates have been quick to blame the database for performance problems. – Jim G. Feb 19 '11 at 23:15
    
Multiprocessing (while IPC may be more complex) are the answer. Why? Because garbage collection is as easy as dropping a process and, if one module of the program crashes, it doesn't bring down the whole system. Look at this google.com/googlebooks/chrome/small_00.html to see what I mean. – Evan Plaice Mar 25 '11 at 12:07
    
@Evan Plaice: You've got to be kidding me. Those performance gains come at the expense of significant programmatic complexity. – Jim G. Mar 25 '11 at 16:35

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

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

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
    
You are certainly correct that those things make unit testing hard, but you are assuming the best practice of unit testing everything. There are other ways to test, there are not other ways to make things static or final if you need those efficiencies / protections. – Bill Oct 27 '10 at 21:13
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

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

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
    
The only similar case I've seen is when using raw MySQL functions in PHP and a while ($row = mysql_fetch_row method. I don't recommend it. – Josh K Jan 13 '11 at 14:18
    
I do this in Java when I am reading from a BufferedReader. That is: for(String line; (line = in.readLine()) != null;){ /*do stuff*/} This is nice since I am only calling in.readLine() once in my code. – Jeremy Heiler Jan 17 '11 at 15:30
    
It's similar to Python's \n for line in open('file.txt','r').readlines():\n\tprint line – Christopher Mahan Jan 20 '11 at 22:19
    
I used to think I was terribly clever doing things like this. Then I saw the light. If I have to stop and have a big pause to figure out whats really going on... then the programmer failed. Its clearer, in the end, to break this into 2 statements / lines. – quickly_now Feb 6 '11 at 10:02

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
    
    
If you do embedded development on small micros, and actually look at the generated code, you will frequently find that using const where you want something to be read-only DOES generated better code. – quickly_now Feb 6 '11 at 10:03
    
@sbi: sure, every technique has some benefits. Unfortunately the end result and productivity is what counts. Why c# is considered to be more productive language than c++? Because there's no const-correctness in it, that's why. :) Just kidding of course.. – AareP Feb 19 '11 at 19:39
    
@AreP: I'm actually programming in C#, and I think the main reason is its rich infrastructure (.NET libraries). As for productivity, nothing is less productive than spending days to hunt down silly bugs: chat.stackoverflow.com/transcript/message/254711#254711 – sbi Feb 19 '11 at 21:36

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