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

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

45 Answers

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

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

I think you've hit the nail on the head with this statement

I'd hate to take things at face value and not think about them critically

I ignore almost all Best Practices when it doesn't come with explanation on why it exists

Raymond Chen puts it best in this article when he says

Good advice comes with a rationale so you can tell when it becomes bad advice. If you don't understanding why something should be done, then you've fallen into the trap of cargo cult programming, and you'll keep doing it even when it's no longer necessary or even becomes deleterious.

share
4  
Wonderful quote. –  David Thornley Oct 26 '10 at 20:10
4  
+1 Great quote and I agree 100%. –  Fosco Oct 26 '10 at 20:25
3  
I hope people don't take this as an excuse to not look up what the reasoning behind the best practices are, though. ;) Unfortunately, I have seen developers with this attitude. –  Vetle Oct 27 '10 at 8:23
3  
Rationale is good. Research is better. –  Jon Purdy Oct 27 '10 at 14:26
7  
Absolutely true, and I've said the same before. Perhaps the first standard in any "Standards" document should read, "In the interest of sharing knowledge and providing the information necessary to rescind standards at later times, all standards shall include the reasons for their existence." –  Scott Whitlock Oct 29 '10 at 0:47
show 3 more comments

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

The sacrifices we make to make code testable

I jump through a lot of hoops to make my code testable, but I don't pretend that I wouldn't if given the choice. However, I often hear people pushing the idea that these are "best-practices." These practices include (written in the language of .Net, but applies to other languages as well):

  • Creating an interface for every class. This doubles the number of classes (files) to deal with, and duplicates code. Yes, interface programming is good, but that is what the public/private specifiers are meant for.
  • Every class not instantiated at startup needs a factory. Clearly, new MyClass() is much simpler than writing a factory, but now the method that creates it cannot be tested in isolation. If not for this fact, I would only make 1/20th the number of factory-classes that I do now.
  • Make every class public, which defeats the purpose of having access specifiers on classes at all. However, non-public classes cannot be accessed (and thus, tested) from other projects, so the only other option is move all the testing code to the same project (and thus release it with the final product).
  • Dependency Injection. Clearly having to give every other class I use a field and a constructor-parameter is significantly more work than just creating them when I need them; but then I can no longer test this class in isolation.
  • The Single Responsibility Principle, which has caused me so many headaches I've moved it to its own answer.

So what could we do to fix this? We'd need a drastic change in the language architecture:

  • We'd need the ability to mock classes
  • We'd need the ability to test private methods of internal classes, from another project (this may seem like a security vulnerability, but I don't see a problem if the testee is forced to name its tester-classes).
  • Dependency injection (or service location), as well as something equivalent to our existing factory pattern, would have to be a core part of the language.

In short, we need a language designed from the ground-up with testability in mind.

share
show 3 more comments

The Single Responsibility Principle

("every class should have only a single responsibility; in other words, every class should have one, and only one, reason to change")

I disagree. I think a method should have only one reason to change, and all methods in a class should have a logical relationship to one-another, but the class itself might actually do several (related) things.

In my experience, this principle too-often gets applied over-zealously, and you end up with many tiny one-method classes. Both the agile shops I've worked at have done this.

Imagine if the creators of the .Net API had had this sort of mentality: rather than List.Sort(), List.Reverse(), List.Find() etc., we'd have ListSorter, ListReverser, and ListSearcher classes!

Rather than argue anymore against the SRP (which itself isn't terrible in theory), I'll share a few of my long-winded anecdotal experiences:


At one place I worked, I wrote a very simple max flow solver which consisted of five classes: a node, a graph, a graph-creator, a graph-solver, and a class to use the graph-creator/solvers to solve a real-world problem. None were particularly complex or long (the solver was by far the longest at ~150 lines). However, it was decided that the classes had too many "responsibilities," so my co-workers set about refactoring the code. When they were done, my 5 classes had been expanded to 25 classes, whose total lines-of-code were more than triple what they were originally. The flow of the code was no longer obvious, nor was the purpose of the new unit-tests; I now had a hard time figuring out what my own code did.


At the same place, almost every class had only a single method (its only "responsibility"). Following the flow within the program was nearly impossible, and most unit-tests consisted of testing that this class called code from another class, both of whose purpose were equally a mystery to me. There were literally hundreds of classes where there should have been (IMO) only dozens. Each class did only one "thing", but even with naming conventions like "AdminUserCreationAttemptorFactory", it was difficult to tell the relationship between classes.


At another place (which also had the classes-should-have-only-one-method mentality), we were trying to optimize a method which took up 95% of time during a certain operation. After (rather stupidly) optimizing it a bit, I turned my attention towards why it was being called a bajillion times. It was being called in a loop in a class... whose method was being called in a loop in another class.. which was also being called in a loop..

All told, there were five-levels of loops spread out over 13 classes (seriously). What any one class was actually doing was impossible to determine just by looking at it - you had to sketch a mental graph of what methods it called, and what methods those methods called, and so on. If it had all been lumped into one method, it would have only been about 70 lines long with our problem-method nested inside five immediately-obvious levels of loops.

My request to refactor those 13 classes into one class was denied.

share
6  
Sounds like someone got "Pattern Fever" or in this case "Principle Fever" at that job. The list class doesn't violate SRP. All of its functions serve one purpose, manipulating a collection of objects. Having only one function in a class sounds like overkill to me. The principle behind SRP is that a unit of code (be it method, class, or library) should have a single responsibility that can be stated succintly. –  Mike Brown Jan 22 '11 at 0:37
3  
I've started to see this kind of madness from people who find it impossible to write pure plain functional code. Too much educating that every problem in the world can be solved out of a book of patterns. Not enough thought about pragmatism. Like you I've seen some class-based OO code thats so horrible that following it is completely impossible. And its huge and bloated. –  quickly_now Feb 6 '11 at 9:51
3  
2nd comment here. Lots of "principles" are over-applied. There are many things that are good ideas, where doing the exact opposite is sometimes appropriate. GOOD programmers know when the break the rules. Because the rules are not "rules", they are statements of "good practice most of the time except when its a really dumb idea." –  quickly_now Feb 6 '11 at 9:53
show 1 more comment

80 char/line limits are dumb

I understand that some compromises need to be made to match the pace of the slowest runner on the GUI side (screen resolution limitations, etc) but, why does that rule apply to code formatting?

See... There was this little invention called the horizontal scroll bar that was created to manage virtual screen space outside of the right-most pixel boundary. Why don't developers, who have managed to create great productivity enhancing tools like syntax highlighting and auto-complete use it?

Sure, there are CLI editors religiously used by *nix dinosaurs that follow the tired old limitations of their VT220 terminals but why are the rest of us held to the same standard?

I say, screw the 80 char limit. If the dinosaurs are epic enough to hack on emacs/vim all day, why can't the should be capable of creating an extension that automatically wraps lines or gives horizontal scrolling capabilities to their CLI IDEs?

1920x1080 pixel monitors will eventually become the norm and developers worldwide are still living under the same limitations with no bearing on why they do except, that's what they were told to do by their elders when they were just starting to program.

80 char limits are not a best practice but a niche practice for a very minority of programmers and should be treated as such.

Edit:

It's understandable that many devs don't like the horizontal scrollbar because it requires a mouse gesture so... Why not increase the column width limit to a higher number (than 80) for those of us who use modern displays.

When 800x600 computer monitors became the norm for most users, web developers increased their website widths to accommodate the majority... Why can't developers do the same.

share
1  
@Orbling Nice GWB logic with the ___ is evil angle. So you loathe the hScroll, do you have any valid reason why col-width should be limited to 80 chars? Why not 160 or 256? I think we can all assume that most developers have retired their VT220 terminals and replaced them with puTTY so they're capable of extending the width programmatically anyway. –  Evan Plaice Jan 18 '11 at 1:38
4  
I prefer that we stick to the 80 char limit. As soon as you give me more horizontal space, I'll try to open up additional documents side-by-side with the rest. I'd hate to have to scroll four ways. Also, I've noticed often that I'm force to write more readable code with the 80 char cap. –  Filip Dupanović Jan 18 '11 at 2:17
2  
how would you print it? –  user1249 Jan 21 '11 at 0:11
5  
Sorry - have to disagree on this one. I really hate long long lines - it requires more eye movement, it requires mouse gestures to scroll across, its harder to see subtly little dangly things down the end of a line. In about 99% of cases there are clean ways of making stuff run over several (shorter) lines which is clearer and easier to read. 80 Chars might be arbitrary and "because it was that way in punch card days" but its still a reasonable framework to work inside - most of the time - for the reasons above. –  quickly_now Jan 21 '11 at 1:25
3  
Indent by 2 spaces. And use an editor with an automatic indentation tracker. I've done it that way for years, no big deal. (Emacs with the right modes helps here.) –  quickly_now Feb 6 '11 at 9:47
show 12 more comments

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

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

"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

Object orientation

There is the assumption, just because code is "object-oriented", it's magically good. So people go on squeezing functionality into classes and methods, just to be object-oriented.

share
7  
I can't imagine building a software system of any significant size without taking advantage of the organization that Object Orientation provides. –  Robert Harvey Oct 28 '10 at 20:55
18  
Robert. Unix isn't object oriented, and it certainly qualifies as a software system of significant size. It also seems to be quite popular (think Mac OSX, iPhone, Android phones, etc) –  Christopher Mahan Oct 29 '10 at 4:29
7  
What i meant is that i think we should use the most appropriate methodology. I've seen people use methods and classes where it cumbersome and does not make much sense, just because "it's object-oriented". That's cargo cult. –  LennyProgrammers Oct 29 '10 at 7:49
8  
There is no silver bullet. Functional Programming (Haskell) is quite successfull without being object oriented. At the end of the day, you've got multiple tools and it is your job to pick the best assortment for the task at hand. –  Matthieu M. Nov 22 '10 at 20:46
9  
The funny thing is, that besides using classes, polymorphism and the likes, most of object oriented code is in fact procedural code. –  Oliver Weiler Nov 30 '10 at 0:05
show 8 more comments

Separating applications into Tiers; Data Layer, Business Layer, UI Layer

The main reason that I dislike this is that most places that follow this method, use very brittle frameworks for getting it done. I.E. UI Layer is hand coded to deal with business layer objects, business layer objects are hand coded to deal with business rules and database, database is SQL and is already fairly brittle and managed by the "DBA" group who dislike change.

Why is this bad? The most common enhancement request is likely "I need a field on screen X that has Y." Bang! You just have a new feature that affects every single layer, and if you separate layers with different programmers, it just became a big issue involving multiple people and groups, for a very simple change.

Additionally, I don't know how many times I've been in arguments that go something like this. "The name field is limited to a maximum length of 30, is this a UI layer, or business layer, or data layer issue?" And there are a hundred arguments, and no right answer. The answer is the same, it affects all layers, you don't want to make the UI dumb and have to go through all layers, and fail at the database, just so the user finds out his entry is too long. If you change it, it affects all layers, etc.

The "layers" tend to leak as well; If any layer is physically separated by process/machine boundaries (I.E. web UI and business backend logic), the rules get duplicated to make everything work reasonably well. I.e. some business logic ends up in the UI, even though it's a "business rule", because the user needs the UI to be responsive.

If the framework used, or architecture used, is resilient to small changes and leakage, i.e. based on meta data, and dynamically adjusted through all layers, it can be less painful. But most frameworks this is a royal pain, requiring UI changes, business layer changes, and database changes, for every small change, and causing more work and less help than the technique is supposed to produce.

I will probably get slammed for this, but there it is :)

share
show 2 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

Methods shouldn't be longer than a single screen

I agree with the single-responsibility principle completely but why do people perceive it to mean "a function/method can have no more than a single responsibility at the finest level of logical granularity"?

The idea is simple. A function/method should accomplish one task. If part of that function/method can be used elsewhere, chop it out into it's own function/method. If it could be used elsewhere on the project, move it into its own class or a utility class and make it internally accessible.

Having a class that contains 27 helper methods that are only called once in the code is dumb, a waste of space, an unnecessary increase in complexity, and a massive time sink. It sounds more like a good rule for people who want to look busy refactoring code but don't produce much.

Here's my rule...

Write functions/methods to accomplish something

If you find yourself about to copy/paste some code, ask yourself whether it would be better to create a function/method for that code. If a function/method is only called once in another function/method, is there really a point in having it in the first place (will it be called more often in the future). Is it valuable to add more jumps in/out of functions/methods during debugging (Ie, does the added jump make debugging easier or harder)?

I completely agree that functions/methods greater than 200 lines need to be scrutinized but some functions only accomplish one task in as many lines and contain no useful parts that can be abstracted/used on the rest of the project.

I look it at from an API dev perspective... If a new user were to look at the class diagram of your code, how many parts of that diagram would make sense within the greater whole of the project and how many would exist solely as helpers to other parts internal to the project.

If I were to choose between two programmers: the first has a tendency to write functions/methods that try to do too much; the second breaks every part of every function/method to the finest level of granularity; I would choose the first hands down. The first would accomplish more (ie, write more meat of the application), his code would be easier to debug (due to fewer jumps in/out of functions/methods during debugging), and he would waste less time doing busy work perfecting how the code looks vs perfecting how the code works.

Limit unnecessary abstractions and don't pollute the autocomplete.

share
3  
I think one counter argument to this is that refactoring parts of a large method into separate calls can make the larger method easier to read. Even if the method is only called once. –  Jeremy Heiler Jan 17 '11 at 15:33
1  
@Jeremy How? How does, abstracting out a section of code and placing it in its own method make it any more readable than just placing a one line comment at the top of that chunk of code describing what it does? Assuming that block of code is only used once in that section of code. Is it really that difficult for most programmers to decompose the working parts of the code while they read it and/or place a few one-line comments to remind them what it does if they can't? –  Evan Plaice Jan 18 '11 at 1:28
4  
@Evan: Putting pieces of code into functions effectively gives them a name, hopefully well explaining what that piece of code does. Now, wherever that piece of code is called, you can see the name explaining what the code does, instead of having to analyze and understand the algorithm itself. If done well, this can ease reading and understanding code tremendously. –  sbi Jan 19 '11 at 19:00
1  
+1 and I'd give more if I could. There's nothing at all wrong with a lump of C code thats 1000 lines in a single function (eg a parser with a massive switch()) PROVIDED the intent is clear and simple. Busting all the little pieces out and calling them just makes the thing harder to understand. Of course there are limits to this too.... sensible judgement is everything. –  quickly_now Jan 21 '11 at 1:22
show 3 more comments

Design Patterns mostly. They are over used and under utilised.

share
13  
+1 I still don't see how Design Patterns are beautiful or elegant solutions. They are workarounds to language deficiencies, nothing more. –  Oliver Weiler Nov 30 '10 at 0:02
2  
Just see it as an endeavor of eradicating language smell using the language itself. –  Filip Dupanović Jan 18 '11 at 2:10
add comment

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

Using unsigned int as iterator

When will they learn that using signed int is much safer and less bug-prone. Why is it sooo important that array index can only be positive number, that everyone is glad to overlook the the fact that 4 - 5 is 4294967295?

share
4  
@Paul Nathan: as far as the buggyness goes here's one example: for(unsigned int i=0; i<10; i++){int crash_here=my_array[max(i-1,0)];} –  AareP Dec 10 '10 at 5:59
1  
@Adam Paynter: yes. This might seem normal for c++ programmers, but let's face it, "unsigned int" is one bad "implementation" of positive-only-numbers. –  AareP Dec 10 '10 at 11:19
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

Use Singletons

When you only should have one instance of something. I cant disagree more. Never use a singleton and just allocate it once and pass around the pointer/object/reference as necessary. There is absolutely no reason not to do this.

share
2  
That's a whole bunch of negatives that has left me rather confused as to your actual stance on singletons. –  Paul Butcher Nov 30 '10 at 11:17
1  
@Paul Butcher: I hate singletons and it should never be used –  acidzombie24 Dec 1 '10 at 7:58
1  
@rwong: Personally i dont think any reason is a legitimate reason. Just write it as a normal class. Really, there is no reason to use a singleton other then lazyness and to promote bad habits or design. –  acidzombie24 Dec 1 '10 at 22:23
6  
Who says that using Singeltons is best practice? –  Phil Mander Dec 10 '10 at 0:29
2  
Singletons do have their place, especially in instances where an operational structure is allocated and filled on startup, then basically becomes read only throughout the run time of the program. In that case, it just becomes a cache on the other side of a pointer, though. –  Tim Post Dec 10 '10 at 8:06
show 11 more comments

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

One return per function/method.

share
7  
I was going to put this. I love me some early return statements. –  Carson Myers Dec 1 '10 at 3:37
4  
Absolutely! People contrive some pretty interesting program flow to avoid an early return statement. Either deeply nested control structures or continual checks. This can really bloat a method when a if return could really simplify this problem. –  snmcdonald Dec 19 '10 at 22:26
4  
If you need multiple returns in a function (aside from guards) your function is probably too long. –  EricSchaefer Dec 28 '10 at 20:16
18  
There's no point in having a return keyword unless it was intended to appear in multiple places. Return early, return often. It'll only serve to simplify your code further. If people can understand how break/continue statements work, why do they struggle with return? –  Evan Plaice Jan 12 '11 at 16:14
7  
I think this is a very out-dated best practice. I don't think it's a modern best practice. –  Skilldrick Jan 17 '11 at 11:26
show 6 more comments

My teacher demands I start all my identifiers (not including constants) with lowercase letters, e.g. myVariable.

I know it seems like a minor thing, but many programming languages require variables to start with uppercase letters. I value consistency, so it's a habit of mine to start everything with uppercase letters.

share
9  
I had a teacher that required camelCase because he insisted that was what people use in the realWorld... At the same time I was programming in two different groups at my work - both groups insisted on under_scores. What matters is what your group uses. He could have defined himself as the lead programmer and all would have been fine in my book - we follow his conventions - but he was always giving his opinion as "the way things are done in the real world" as if there was no other valid way. –  xnine Nov 30 '10 at 4:42
5  
Camel case (first letter lowercase) is a pretty common convention as well as Pascal case (first letter of every word capitalized). In most languages, camelCase is used on private/internal variables where PascalCase is used on classes, methods, properties, namespaces, public variables. It's not a bad practice to get used to just be ready for projects that may use a different naming scheme. –  Evan Plaice Jan 12 '11 at 16:18
2  
Just an FYI. Some languages infer meaning from whether the first letter of a variable is capital or not. In one such language, if the first letter is capital the variable is treated as a constant--any attempt to change it will throw an error. –  Berin Loritsch Jan 12 '11 at 18:28
1  
In Go, public methods and members in classes start with an Upper-case letter; private ones with a lower-case letter. –  Jonathan Leffler Jan 18 '11 at 14:39
show 3 more comments

User Stories / Use Cases / Personas

 

I understand the need for these when you are programming for an industry that you are not familiar with, but I think that when they are implemented in full force they become too corporate and become a waste of time.

share
add comment

Measure, Measure, Measure

So fine, measure away, but for isolating performance bugs, measuring works about as well as successive elimination. Here is the method I use.

I've been trying to track down the source of the measure-measure "wisdom". Somebody with a tall enough soapbox said it, and now it travels.

share
add comment

Methodologies, particularly scrum. I can't keep a straight face when I hear grownups use the phrase "Scrum Master". I get so tired of hearing developers protest that some aspect of Methodology X isn't working for their company, only to be told by Guru So-and-So that the reason it's not working is that they are not, in fact, true practitioners of Methodology X. "Scrum harder, you must, my Padawan learner!"

There are nuggets of wisdom in agile methodologies---lots of them---but they're often couched in so much manure that I can't fight my gag reflex. Take this bit from Wikipedia's scrum page:

A number of roles are defined in Scrum. All roles fall into two distinct groups—pigs and chickens—based on the nature of their involvement in the development process.

Really? Pigs and chickens, you say? Fascinating! Can't wait to pitch this one to my boss...

share
12  
+1...you're right, its hard to take that seriously. <big booming voice> *I AM THE SCRUMMASTER*</voice> –  GrandmasterB Nov 29 '10 at 21:50
2  
And the parables. It reminds me of church sermons, or the kinds of anecdotes that self-help gurus (and comedians) are famous for: "Take my friend Steve. Steve was constantly arguing with his wife Sheryl. Those two would go at it for hours, to the point where their marriage was in real jeopardy. Then, one day..." These kinds of didactic yarns don't bother me in other spheres, but I hate to see them proliferate in the engineering sciences. –  evadeflow Nov 29 '10 at 22:02
2  
What about the Scrum Ninjas? –  Berin Loritsch Jan 12 '11 at 18:20
1  
I don't agree with the "Pig and Chicken" comparison...it flies directly in the face of the Agile Manifesto. Namely "Customer collaboration over contract negotiation". Customers are as vested (if not moreso) in the success of the project as the project team. Calling some roles pigs and other roles chickens sets up an "us versus them" mentality that IMHO is the biggest roadblock to successful projects. –  Mike Brown Jan 21 '11 at 23:58
show 3 more comments

All code should be commented.

No, it shouldn't be. Some time you have obvious code, for example setters should not be commented, until they do something special. Also, why should I comment this:

/** hey you, if didn't get, it's logger. */
private static Logger logger = LoggerFactory.getLogger(MyClass.class);
share
13  
All code should be understandable. Comments are a major tool in that, but far from the only one. –  Trevel Jan 17 '11 at 22:27
2  
Generated documentation. That's what the /** */ format is for instead of the /* */ format comment. Or for .NET it would be /// –  Berin Loritsch Jan 18 '11 at 16:19
10  
Using }//end if, }//end for, }//end while are the best example of wasteful commenting I've ever encountered. I've seen this many times where the opening brace is no more than 2 lines above. IMHO if you need these comments then your code needs re-factoring... or you need to pony up $20 and buy an IDE/Text editor that highlights matching braces. –  scunliffe Jan 21 '11 at 11:54
7  
Code say "how". Comments need to say "why". –  user1249 Jan 27 '11 at 18:03
show 4 more comments

"Be liberal with comments"

Comments are definitely a good thing, but too many are just as bad if not worse than not enough. Why? Well, people tend to tune comments out if they see too many unnecessary ones. I'm not saying that you can have perfectly self-documenting code, but it is preferable to code that needs comments for explanation.

share
1  
self-documenting code is definitely nice. Although, I like having comments along-side something like simple calculations (to express what the return-type or returned-value is). However, if your comments need more words than the code itself, it's probably time to rewrite the code. –  sova Nov 29 '10 at 20:11
6  
I have to agree with the way that sova put this one. Clean code is preferable to comments. –  Stargazer712 Nov 29 '10 at 23:47
4  
You still need the "why"'s in there! –  user1249 Jan 21 '11 at 0:04
show 1 more comment

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

YAGNI

(You ain't gonna need it)

This approach has cost me hours and hours when I had to implement features on an existing codebase, where careful planning would have included these features beforehand.

Ideas of mine have often been rejected because of YAGNI, and most of the times someone had to pay for that decision later.

(Of course one could argue that a well-designed codebase would also allow features to be added later on, but reality is different)

share
15  
I agree with YAGNI, but I see what you're getting at. The point of YAGNI is to deal with people who want to plan everything up front down to the slightest detail. Nine times out of ten though, it's used as an excuse to under engineer code or completely skip planning. –  Jason Baker Nov 29 '10 at 19:55
12  
P.Floyd, @Jason Baker: +1 Absolutely right. The old saying applies here "months in the lab can save hours in the library" –  Steve Evers Nov 29 '10 at 21:40
1  
@TokenMacGuy the crucial aspect is the implied by the spec part. That's where opinions differ a lot. –  Sean Patrick Floyd Jan 28 '11 at 14:04
show 1 more comment

GoTo Considered Harmful

If you are implementing a state machine a GOTO statement can make more sense (readability and efficient code) than a "structured programming" approach. It really worried some co-workers when the first piece of code I wrote in a new job included not just one but several goto statements. Fortunately they were intelligent enough to realize that it was actually the best solution in this particular case.

Any "best practice" that doesn't allow for sensible and documented exceptions to its rules is just plain scary.

share
16  
I'm going on nine years programming without a single goto statement (including several state machines, as you mentioned). Expand your mind to new ideas. –  Stargazer712 Nov 22 '10 at 4:46
3  
@Mathieu M. - agreed - mixing GOTO with structured control statements isn't sensible. (This was pure C and it wasn't an issue. –  MZB Nov 25 '10 at 0:22
1  
@Stargazer2 - with a simple FSM, it depends whether putting the state in a variable and using it as an index to call a procedure (isn't that the same as a computed GOTO?) gives clearer/faster code than using the program counter as the FSM state. I'm not advocating this as the best solution in most circumstances, just the best solution in some circumstances. Expand your mind to alternative approaches. –  MZB Nov 25 '10 at 0:31
5  
@MZB, wouldn't you agree that a function call is also just a computed GOTO? Same goes for for/while/if/else/switch constructs, among others. Language designers abstract away direct changes to the program counter for a reason. Do not use goto. –  Stargazer712 Nov 29 '10 at 22:37
3  
Directly implementing a statemachine is probably an antipattern. There are lots of ways to have a state machine without expressing the states and transiitions literally. for instance, import re –  TokenMacGuy Jan 19 '11 at 2:37
show 10 more comments

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