98
votes

"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.

14
  • 8
    Which are the practices you disagree with?, just curious. Commented Oct 28, 2010 at 13:17
  • Anyone can write a book, and I do not have to agree with it - it is as simple as that.
    – Job
    Commented Jan 12, 2011 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
    Commented Jan 17, 2011 at 14:44
  • 5
    @Walter: This has been open for months, it is definitely constructive, why close it?
    – Orbling
    Commented Jan 18, 2011 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
    Commented Jan 19, 2011 at 1:33

45 Answers 45

1
2
2
votes

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

1
  • 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). Commented Jan 18, 2011 at 2:01
1
vote

Over generalization.

The square block does NOT fit into the circle hole!

5
  • 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. Commented Oct 26, 2010 at 20:08
  • 6
    Since when was "over generalization" a best practice? Commented Oct 26, 2010 at 20:59
  • Seems more like an antipattern I guess.
    – PMV
    Commented Oct 27, 2010 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
    Commented Oct 27, 2010 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
    Commented Oct 27, 2010 at 15:59
1
vote

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.

12
  • 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.
    – riwalk
    Commented Nov 22, 2010 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
    Commented Nov 22, 2010 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.
    – riwalk
    Commented Nov 29, 2010 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
    Commented Nov 29, 2010 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
    Commented Jan 17, 2011 at 22:32
1
vote

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.

2
  • 2
    I like if( condition ) { Commented Dec 1, 2010 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.
    – user131
    Commented Dec 10, 2010 at 8:02
1
vote

"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.
3
  • 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." Commented Jan 17, 2011 at 19:49
  • 2
    You forgot: The original coder is dead. Commented Jan 20, 2011 at 22:16
  • 3
    If the code is proven to match the requirements, most likely the requirements are broken. Commented Jan 21, 2011 at 8:37
1
vote

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: https://stackoverflow.com/questions/3282303/my-choice-of-class-names-is-hampered-by-windows-xp-max-path-length-issues-with-sv

2
  • 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. Commented Jan 18, 2011 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
    Commented Jan 21, 2011 at 17:00
0
votes

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.

7
  • 2
    Douglas Crockford says to put the semi-colon, so I put the semi-colon. Commented Nov 30, 2010 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."
    – Tyler
    Commented Nov 30, 2010 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. Commented Nov 30, 2010 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.
    – riwalk
    Commented Nov 30, 2010 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? Commented Nov 30, 2010 at 15:45
0
votes

Databinding

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

1
  • 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? Commented Dec 10, 2010 at 16:06
0
votes

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.

3
  • 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.
    Commented Feb 19, 2011 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. Commented Mar 25, 2011 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.
    Commented Mar 25, 2011 at 16:35
0
votes

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.

0
votes

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.

-1
votes

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.

10
  • 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
    Commented Oct 27, 2010 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 Commented Oct 27, 2010 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. Commented Oct 27, 2010 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. Commented Oct 28, 2010 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). Commented Oct 28, 2010 at 15:05
-1
votes

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.

9
  • 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. Commented Oct 28, 2010 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 Commented Oct 28, 2010 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). Commented Oct 28, 2010 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. Commented Oct 28, 2010 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. Commented Nov 1, 2010 at 16:19
-3
votes

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.

6
  • 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. Commented Jan 12, 2011 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
    Commented Jan 13, 2011 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
    Commented Jan 17, 2011 at 15:30
  • It's similar to Python's \n for line in open('file.txt','r').readlines():\n\tprint line Commented Jan 20, 2011 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. Commented Feb 6, 2011 at 10:02
-8
votes

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>();
5
  • 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
    Commented Jan 12, 2011 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. Commented Feb 6, 2011 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
    Commented Feb 19, 2011 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
    Commented Feb 19, 2011 at 21:36
1
2

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