Well, "good practice" and "bad practice" are tossed around a lot these days - "Disable assertions in release builds", "Don't disable assertions in release builds", "Don't use goto.", we've got all sorts of guidelines above and beyond simply making your program work. So I ask of you, what coding practices do you violate all the time, and more importantly, why? Do you disagree with the establishment? Do you just not care? Why should everyone else do the same?

cross links:

What's your favorite abandoned rule?

Rule you know you should follow but don't

+8  A: 
for(int i=0; i<20; i++) {
    someFunction("magic string literal");
  • java style braces
  • 1-letter loop variables
  • literals in code
  • spaces instead of tabs
I think the string literal is the only thing wrong here. The brace style is debatable, but I like 1-letter loop variables, and I *strongly* encourage spaces instead of tabs.
Bill the Lizard
Yeah - brace style is a matter of, um, style. =)
Erik Forbes
change your tab key to be spaces instead of tabs. That way you can easily keep your indentations consistent in all editors.
@Nailer: That's how NetBeans is set up by default. I think Eclipse is too, but my memory is failing...
Bill the Lizard
Agreed with Bill the Lizard: The only "bad" thing here is the string literal ... Everything else is highly debatable. (And count me on the spaces side -- tabs jack everything up when you switch to another editor!)
John Rudy
The convention of using 'i' as a loop counter is so universal, I think it's a bad practice *not* to do so.
Alan Moore
sadly, "no 1-letter variables" is policy at a lot of places (to avoid any "well, this is *technically* just a temporary counter variable) slippery-slope temptations I guess)
I avoid 1 letter variables if I'm doing nested loops. i looks a lot like j, and that in and of itself can cause lots of needless yet hard to catch bugs.
Jason Baker
@John Rudy: maybe it's because your other editors are broken? Tabs are tabs, they're configurable, and you're able to change the way they appear without changing the source file. (note: not starting a flamewar here :P)
Jorge Alves
I always use 2-letter loop variables, like "ii". Concise, while still being easy to search for instances of.
+68  A: 

I reinvent the wheel. Every Time.

Because I like to know 'how stuff works'.

I do look at how other people solves the same problem. But in the end, I rewrite the code myself and try to learn something new along the way.

  Never roll your own crypto!
  You can't tackle i18n on your own.

This goes along with what Joel stresses. Find the dependencies and eliminate them. If it's a core business function, do it yourself. And probably a few other quotes I'm forgetting.
+1 for me-too. I find it horrifying when I see people telling others to use a 100KB javascript framework for a one-liner just because they can't be bothered learning to say "give me all links with this class name" in JS themselves.
Ant P.
Often true but this runs the risk of assuming you know everything and others know nothing. This has led to people rewriting things unnecessarily on projects I've worked on. It's a double-edged sword.
Jeff Yates
ffpf: amen.A perfect example is security and cryptography. The same developer who says, "Write everything yourself!" will also say, "Never roll your own crypto!"
Lucas Oman
This also leads to people using the same techniques to implement something across multiple platforms. You have no idea how many times I've seen Java-isms or C#-isms forced into Python.
ow, in addition to "Never roll your own crypto!" I would like to add "Never try to tackle i18n yourself"
I do this sometimes too, +1
That i18n exception is soo true...
Alix Axel
Threads are another thing that it's best to avoid reinventing. Just too many nasty cases that only show up in production. Use someone else's debugged threading primitives, please!
Donal Fellows
+2  A: 

I document asymetrically. Classes and interfaces which go public I document thoroughly and extensively. On the other hand, code I know no one will ever see I barely document at all.

Yuval A
+8  A: 

Well, the best case is, to do it all wrong once.

Nothing beats own experience.

Just don't do it in a bomb factory, because you'll only have one chance.
Cristián Romo
If at first you don't succeed, maybe skydiving isn't for you
Mike Robinson
+22  A: 

I hack. Occasionally when needs must and time-frames are short you have to add in cute tricks as opposed to completely restructuring a massive swathe of the system.

We had race conditions appear in a certain critical part of the application and to remove those "properly" we would have needed to modify a large amount of APIs and we needed to release a version to test for the next day. The answer? A globally available lock.

Yes, it's bad practise, yes its a hack but yes we also met our schedule. The globally available lock is heavily commented and scheduled for a tidy later on.

Hacks ain't always bad.

And even when they are bad, there is such a thing as acceptable evil. Getting Things Done is more important than perfection.
Erik Forbes
Amen to Getting Things Done. I'd love to be able to write pure, perfect code -- but clients don't ever want to pay for it, nor do they want to wait for it. As long as most of the code is good, the occasional hack (well-documented, as described) is an acceptable evil.
John Rudy
FWIW, that's a hack that's used by lots of programming languages (like Ruby or Python), so you're in good company. :)
Jason Baker
haha, i thought that's acceptable in agile right, because eventually we need to refactor codes anyway. but does anyone have a good way to manage all the snippets that you've done?
+27  A: 

I do Unit Test after the whole section of code work. I do not do them before, not even in the same time. Once it works, I do unit test... and some time, I just do not have the time to Unit Test everything...

For shame! I do this too. :)
Bill the Lizard
Copy cat - and -
Haven't see that post, I would have close this one if knew it sorry
+30  A: 
if (c==0)

instead of:

if (0==c)

Just because the second one looks really goofy to me. Modern compilers warn you anyways...

Brian Knoblauch
What's wrong with the first one? It reads better. Are you coding in C, where the compiler won't warn you if you mistype as C=0
Not bad practice.
no, not at all. I would think that the second example, while not wrong, is worst than the first.
Ed Swangren
"worse", not "worst"...
Ed Swangren
The second is usually preferred because you will usually get an error, not just a warning, if you try to assign to an rvalue.
Kris Kumler
Worked on a project where they insisted on using the 2nd form. Drove me nuts to read the code.
treat warnings as errors instead. Yoda Conditionals are hard to subvocalize, therefore create more cognitive load on the reader.
Pavel Radzivilovsky
+86  A: 

I break the "One exit point from a function" rule to enable the use of "Guard Clauses" Example:

   String doSomething(String param1, Object param2) {
      if (isEmpty(param1)) return "";
      if (param2 == null) return "";

      // rest of method
      return result;

Most of the time I would throw and exception if the arguments are illegal, but some of the time a return statement is fine...

Bent André Solheim
The amount of times i've fought against the "no multiple return points" dogma..... *sigh*. +1
I really prefer guard clauses.
Bill the Lizard
It's not really an official bad practice. It was but not anymore, maybe it depends of the language.
That is more like a 'best practice' if you ask me :)
void String doSomething?
Vinko Vrsalovic
No, it is still considered by some devs as bad practice and religiously so... well at least some of the ones at my work..... :(
I like these for argument validation at the top of a function. If I see one way down in the middle somewhere it makes me nervous.
Joel Coehoorn
In many cases doing this make the function simple to understand and trace - and that's *always* best practice no matter what the rules say. +1
As long as your functions are not too long, that rule is no longer relevant. Especially if your language supports RAII or garbage collection.
It's not a rule.
I don't think this is a bad practice at all, as long as your methods are small and easy to understand. In fact, I've read API guidelines encouraging this to avoid nasty multi nested if statements and such.
Ricardo Villamil
I agree with the comments here. I consider the "Arrow AntiPattern", that often occurs when one dont use guard clauses, a bad practice. Guard clauses are good practice, but I know that many consider not obliging the one exit point "rule" a bad practice, so I posted this answer.
Bent André Solheim
I actually think of this as good design. You're not wrapping thing in if statements if your original condition are true, so the code is easier to read. I believe this rule developed in non-managed languages, since most people freed their memory at the end. Early exits could give non-optimal results.
I do this lots in small methods, nothing wrong with it in my book
+4  A: 

In all of my complex UIs in Flex, .Net and Java Swing, I always use a central Universe object which is a Singleton. It's an easy way to get at all pieces of the application.

Yikes. You live on the edge. =)
Erik Forbes
I like the name you gave it. Universe, 'that which contains everything' :P
Erik van Brakel
+1 for the name =)
+14  A: 

In all creative human endeavor, there seems to be a cycle of three phases:

  • Random experimentation. (results wildly hit or miss)
  • An establishment of the "rules" (results solid, but lacking creative edge)
  • Top artists, having mastered the "rules" now know when & where to break them (maximum artistic result)
  • then, lesser artists, who have not mastered the rule, break them randomly, and the cycle begins again.

(In Rock music, those phases roughly translate to the 60s, 70s and 80s (and then the 90s)

The point of this is to say, that once you has mastered the rules, all should be followed, until the exact time when it is best to break one, and at that time, any rule could be broken.

UPDATE: I've expanded on this theory on my blog.

James Curran
+1. I see that pattern (know when to break a rule) here on SO with the "how to parse HTML with regex" questions all the time. Some say "never do it", but really it's "avoid it unless you know what you do". Knowing the difference is key.
Love the pattern ... But seriously, the 80s as top artists in rock? You are kidding, right? :)
John Rudy
1970s punks ignored the rules. Otherwise, good analogies.
But rock and roll has been going downhill since Buddy Holly died. So your analogy falls apart pretty early.
Adam Jaskiewicz
+10  A: 

I (ab)use Perl one-liners:

my $cdata = {map {shift @$_, {map {$_ => 1} @$_}} map {[split /[:,]/, $_]} split ';', delete $config->{Entry}};

I know that it makes my code unmaintainable by other developers, but I love the intellectual challenge off doing the most work in a single line.

Securing our own job are we? =D
You should code for the benefit of the guy who comes after you, not to practice your Jedi Mind Tricks.
Paul Tomblin
Nah, that's easy to understand once you just break it up over several lines, indenting in such a way that statements of the same syntactic level align.
This is precisely the sort of code that turns people (myself included) away from the Perl language - too many Perl programmers are more interested in proving themselves than making something understandable. I can't count the number of Perl 'tutorials' that feature code like this. =(
Erik Forbes
I'm with you on this one - but in c#. Since Linq came around, it's pretty easy.
oh that's a real nasty one !
call me Steve
Hello? Could you please stop downvoting this? It is exactly on topic!
Ummm, you're missing a }
@derobert: Good spotting, I missed it when copying/pasting/anonymising. Fixed up.
Gotta say, the intellectual challenge of writing one-liners in Perl isn't really that great, it's usually a matter of writing readable code, then copy/pasting it onto a one-liner with some variable renaming. The real intellectual is the one who rewrites this later using 3-5 readable lines.
Adam Bellaire
This is just functional programming at work. Look at how I would break this up for readability at the end of this post: [link]( This is not scary at all.
It's not really a one liner is it, it's just lots of bits you have put on the same line. We could all do that why not go the whole hog obfuscate your variables and remove all white space.
I won't downvote this, but for the sake of the guy after you, please stop doing that.
That's the path to insanity.
Eric W.
I must say your spot on with regards to bad practice. If I had to maintain your code I would rather have a bullet through my head. This is the reason I don't do perl.
+3  A: 

I use goto to emulate a labeled break in c++.


I'm normally real big in to not using tables for layout in html.

However, our have to fill out a lot of paper forms for submission to goverment and insurances, and sometimes there are strict rules about using an exact copy of the forms, rather than a reproduction. Of course, we create reproductions anyway, and so these reproductions need to follow the original very closely. In this one and only case, I just fall back to tables for layout.

Joel Coehoorn
+3  A: 

oh dear...

I commit "broken" builds. For pragmatic reasons mind you...

I develop on (at least) two machines - my home machine and my work machine. When it comes for time to going home, I commit my work which goes onto our production server and check it back out when I get home and carry on. Sometimes this is in a development branch, but often not. However:

  1. we're a very small team

  2. we're not working on code that "builds" per se - it's web application development

  3. we tend to be working fairly atomically - if I'm working on a particular controller it's unlikely other members of the team are also working on it

So in practice it rarely causes a problem. It's one issue, that though we've looked at distributed version control systems (I like the look of Bazaar), we will always need to be able to push the things we're working on somewhere that we can get to them from elsewhere...

It depends how you define 'broken' : - doesn't work the way you intent ?- doesn't pass the unit tests (if any ? )- doesn't compile ?If the two first are allowable, the third is unacceptable
Johan Buret
+1  A: 

I use public for most class instead of protected or internal. I think it's because I got lazy and all my unit test for this project is outside the DLL so I have too but well, instead of changing everything... I use public class...

+38  A: 

Start coding before a "design" is completed.

Working code is a lot easier to judge and review than a bunch of box-and-line diagrams drawn by someone who is too important to write code anymore.

And the sooner you write something, the sooner you can re-write it to be the way you should have written it in the first place (and no, an up-front "design" does not eliminate the need for this).

Kristopher Johnson
+1 Depending on what you are designing/developing, coding can be the best way to design. I often find myself writing a unit test just to experiment with different API designs. The unit test is the first client, and you discover awkward constructs other ways of designing would not reveal.
Bent André Solheim
yes i agree, sometimes i just can't get a feel of what the app will be like without seeing a line of code.
Ofcourse box-and-line diagrams drawn by someone who writes code daily really sets you on the right track.
It's ok if you call it prototyping or de-risking.
I may just be bad at designing, but I tend to find lots of little issues when I start writing my apps that I would never have thought of at design time. Especially true when complex db queries are involved. Simple design, bit of coding, back to design if needed.
+5  A: 

First Code, then think.

And I don't refactor/rename/restructure often enough

+1  A: 

In Java, I have difficulties with reusing an object. I too often just throw it away and create a new one instead.


For my web projects, I wrote my own session handling module (MySQL store), in which I threw in other stuff including the kitchen sink, instead of using the vetted session handling modules from CPAN. I did this because:

1) I had some issues (all me, I'm sure) getting some of the other modules to work at the time 2) I was still learning session handling itself at the time and sort of needed/wanted to step through the process 3) I wanted my 'Session' module to handle a ton of other nasty details, such as login screens, account registration, etc. 4) I wanted a single call in my main application to suffice for all of this

Some day, I should probably replace the pure session handling functionality with something more standard, and separate out my other functionality into other modules... but this "bad practice" did accomplish what I set out to do, which was hide a whole slew of nasty details from my main application code and make them reusable across apps.

+2  A: 

Whenever I am allowed to, I use nonstandard indentation and brace style in curly languages. I believe that almost any style is OK, as long as some care is taken to point out the different statements' relation.

  • For me, every statement ends with a semicolon, including after closing braces.

  • For me, an if statement has four arguments: a condition, a clause, and optionally the else keyword followed by an else clause (in my opinion, the else keyword is superfluous, by the way). These four statements are aligned to the same column:

    if (condition)
       { some;
         instructions; };


    if (condition)
       { some;
         instructions; }
       { something;
         different; };
  • If I believe that some code needs more space between lines, I add blank lines.

edit: Please note that this formatting is very consistent: all parts of the same syntactic level are indented the same. Note the symmetry between (condition) and else. Please, also take a look at the Wikipedia article about Indent to see some other styles that are used.

edit: Another advantage is that in this style, I can consistently format longer compound statements, like for example what nrich presented:

my $cdata = { map { shift @$_, 
                          {map {$_ => 1} @$_} }
                  map {[split /[:,]/, $_]}
                      split ';', delete $config->{Entry} };

See? No need to get scared.

If I had to work on your code, I would reformat it it first.
Skip Head
Yeah, that's the difference: I don't have to reformat other's code, since I can see through any sensible formatting style.
There's nothing "sensible" about this - and you have misunderstood the structure of if-else.
It's a bit odd admittedly, but I don't see anything that makes this unreadable. Although, it might end up being a pain to maintain.
Jason Baker
By the way, if you think that this is bad practice, you should UPVOTE it in the light of the question.
Jason Baker, yes, "odd", which just means "unfamiliar". I don't see the pain.
Draemon, I am curious about how you would explain that structure.

I make most classes and methods public to facillitate easy unit testing.

I am trying to get into the TDD spirit, but in complex cases I keep going back to changing everything at once and then fix the unit test at the end.

+2  A: 

I use tables for layout sometimes in HTML, I can't ever seem to get css layouts to work right. But I'm a programmer, not a fancy pants designer. :)

And I constantly forget to use stringbuilder in C# instead of +=

Jeremy Reagan
+36  A: 

I use tables for layout in HTML.

Guilty. I find them easier to maintain and manipulate, and I can (almost) always make them work in all major browsers with little tweaking.
Michael Itzoe
It's not really a case of "making them work" it's a usability issue and not symantecally correct. Tables should only be used to display tabular data and nothing else. Try running your site through a screen reader and see if your web page reads well?
+12  A: 

Guilty secret here. I regularly create Regular Expressions (no pun intended) and don't comment them. I then spend the next week trying to work out what the heck it was that I created, before adding the comments in afterwards. I always say to myself "This time is different. This time I will remember what it actually does." One of these days, I'll get out of that bad habit.

Pete OHanlon
I don't think that's a bad habit unless you know well that it's going to be difficult to dissect. I tend to use those "what the hell was I thinking" moments as a judge of what should and shouldn't be commented. It's served me better than trying to guess ahead of time.
Jason Baker
+1 enjoyed that.
+2  A: 

The most fundamental thing concerning stuff like "best practices" is that you should know what you're doing. If you aren't sure, stick to the "best practice". If you are sure that you fully understand the reason for one of these "best practices" and the consequences, you can vary depending on how you like or the situation requires.

Take GOTOs: The reason for them being banned is that wild GOTOs can make programs difficult to understand and get into an undefined state easily. But especially in C/C++ that lack local functions, it is usually accepted to use them in error handling and cleanup code.

Assertions in production code are a question of what you want: Do you need a correct result (say, accounting) or do you need maximum performance (say, 3D games, ...). For my part (application software, frontend and backend), I've made good experiences with letting them in the code. If the programming language and compiler support that, overflow checking, array range checking and stuff like that should also be enabled, at least during testing. I know cases where checks in production code helped finding errors that occured after years of operation without problems. Some of my customers have the company standard of even to disable compiler optimisation and leave debug code in, although both causes a huge performance "poenality", because correctness and reproducibility, also possible analysis of core dumps, is more important for them than maximum performance.

In my view, the most important task in software development is the reduction of complexity. In most cases it is OK to accept a performance downgrade if it can improve the reliability and maintainability of software.

A point where not too many compromises should be made is error handling. The very least thing you want to do is check for all relevant errors (e.g. return codes) and give out a message where exactly an error occured and with which parameters, ... This can be a lot of work (some statistics claim that error handling can make up more than 80% of a program), but you save most of the time spent there later during debugging.

agreement re 'goto' -- i once tried to sidestep the whole 'for(;;)' vs 'while(true)' debate by coding 'deliberately_hang: goto deliberately_hang' -- *no*body was amused, though it seemed perfectly legit.
+3  A: 

I like brace-less if-statements. I know it is bad but I love the simplicity of:

if (param == null) throw new ArgumentNullException("param");
It is not bad at all if you keep it on one line.
It's not bad until someone maintaining your code down the line adds functionality and then is confused because the behavior isn't expected. "My logic is running all the time! What kind of semicolon did you use here?!?" Braces ensure that control logic is correctly surrounded.
@harlequin: it is bad because nothing is guaranteed to *stay* one line
Well, if someone changes it from single line to multiple lines without adding the block delimiters, then he is to blame for anything that happens afterwards, not the one who wrote the original one-liner.
I agree with you there. If someone changes it to two lines, they must have passed were the braces *should* be, and checked it themselves.
Cristián Romo
+1  A: 

I use var all over the f!#@ing place because I'm lazy as sin.

+8  A: 

I commit changes to source control that combine complex logic changes along with unrelated, wide-sweeping stylistic changes that don't modify the affected code's behavior. As I'm making changes to existing (legacy) code, I find myself peeking at neighboring functions, etc. and can't help but thinking that I should just tweak that code while I'm there--perhaps making it more consistent, perhaps making it easier to read/follow.

This is a less than great idea, but I continue to find myself doing it, perhaps almost compulsively. It has ended up causing pain for me, particularly when I've needed to get peers to review the overall changes or when I've needed to submit the changes for approval. Either the reviewers curse me for causing them to pore over unnecessarily large diffs, or the change actually gets rejected by a fearful manager/reviewer who fears that it's too risky. After these changes get checked in, it also causes confusion to anyone perusing source control history--perhaps making it easy for them to miss the interesting change amidst the Grand Re-tabification.

When in doubt, don't do as I do. Check the changes in separately :)

oh god ... i do this ALL the time
Some diff tools can help with ignoring the "Grand Re-tabification".. WinMerge is pretty good with the appropriate settings.
Greg Ogle
I'm doing this with our purchased source now. 3,200 FxCop errors, 2,400 FxCop warnings, 13,641 StyleCop violations. That is with some of the checks of each turned off.I do do it in a different branch, sometimes. When I remember. ;v)
I edit multiple things at a time, but because I use git for everything, I can still commit (and test!) the changes separately. FTW.
Xiong Chiamiov
+1  A: 

i like to litter my codes with comments, sometimes a little too excessively. as i usually much prefer to read english statements than code...

but nothing too much, just a one liner which tries to explain my intent as human readable as possible...

but i do try to maintain readable code, i.e. give good names to variables, functions, but i usually try to comments some as well.

as long chunks of codes without comments seems scary..not to mention boring :P


I use if statements as if they were case statements, because I prefer the flexibility of elseif statements, but require the ability to interchange parallel statements freely.

elseif(fname = 'Ludwig'){ DoLudwig(); }
elseif(fname = 'Gertrude'){ DoSomethingElse(); }    
elseif(fname = 'Monroe'){ DoSomethingMore(); }
elseif(fname == '' and age > 80){ DoSomethingOld(); }

This points to my personal peeve against the lack of an elseif equivalent that does not require a leading if to begin with. Something like coop_if or otherwise.

For .NET languages, the MSIL actually changes case statements to this if/elseif construction.
+7  A: 

Copy and paste programming. I tell my self I will refactor it later but usually don't unless it bites me in the ass repeatedly.

I do this, usually I need a method and I know I have a similar one in another program. So I get that and modify it a bit. If I spent a bit more time I could probably make it more generic and put it in a dll for use by all my progs. Never do find that time though.
+2  A: 

lock (this) { ... }

I know it's wrong - I worked on CLR/.NET reliability. Someone else can lock on my public object and can therefore corrupt my locking strategy. But I hate allocating a new object for synchronization. Don't tread on my object!

Dino Viehland
+2  A: 

I use SCREAMING_CAPS for my constants in C# (because I find it makes my source easier to read). I also use multiple returns per function where the alternative would complicate the flow of my code (I try to avoid lots of nested ifs whenever possible). Finally I'm guilty of coding before the design is fully complete, but I find this is the best way to proceed in the majority of cases, because my team runs an agile methodology and often many factors are only discovered after coding begins, so it often feeds back into the design process.

Great question by the way :)

Dave R.
+10  A: 

I can't leave messy code alone. Even if it works perfectly and even if i should be doing more productive work.

Turning spaces into tabs, fixing brace positions, adding spaces between code lines where i feel it fits. Sometimes i waste way to much time doing just this.

Ólafur Waage
Aptana + (Ctrl Shift F). Autoformat for the win.
Mike Robinson
Hey, that's me!

For hand-coded lexers, I use gotos.

For algorithms with no structured control flow whatsoever (like state machines and lexical analysers), there really is no sense in contorting your code to avoid them. Those of us who implement such things generally know and accept this (take a look inside the output of lex sometime). However, we don't like to bring it up publicly because to say such aloud is heracy.


In Python, I stack multiple If statements with no elif/else/finally blocks to simulate a case-statement fall-through.

If it helps in my defense, I always comment the start with the purpose and put a footer comment to show where it ends.

If it helps. :S

J.T. Hurley
+2  A: 

I often reinvent the wheel, partly becaus it's nice to know how stuff works, and partly because I'm suspicious about third party code.

+1  A: 

I can't be the only one who makes their member variables public rather than having getters and setters with nothing in them.


I like writing puzzle-solving programs in my spare time. The bad habbit is: I don't finish them. While writing the code, i come up with algorithms, and when the algorithms seems complete, i loose interest in the program itself and start coding another one.

+1  A: 

I have a tendency to reformat (indenting, whitespace, etc.) large swaths of code; messy formatting hurst my head. I never change the actual functionality, but it really throws off source control.

Michael Itzoe
+1  A: 

When declaring variables, i need to have the variable names vertically aligned, so i can end up with something like

int                          i;
StringBuilder                sb;
VeryLongStupidClassName<Foo> baz;

And, worst of all, i do it WITH SPACES.

Evil, I know.


Commenting too much.

I do that because I frown upon undocumented code.

Also, I started this practice years ago - and I think people would think that Mr. T. Complete has become lazy, negligent or whatever if I would change that.

Turing Complete