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:
for(int i=0; i<20; i++) {
someFunction("magic string literal");
}
I reinvent the wheel. Every Time.
why?
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.
Exceptions:
Never roll your own crypto!
You can't tackle i18n on your own.
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.
Well, the best case is, to do it all wrong once.
Nothing beats own experience.
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.
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...
if (c==0)
instead of:
if (0==c)
Just because the second one looks really goofy to me. Modern compilers warn you anyways...
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...
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.
In all creative human endeavor, there seems to be a cycle of three phases:
(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.
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.
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.
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:
we're a very small team
we're not working on code that "builds" per se - it's web application development
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...
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...
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).
First Code, then think.
And I don't refactor/rename/restructure often enough
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.
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; };
or
if (condition)
{ some;
instructions; }
else
{ 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.
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.
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 +=
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.
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.
I like brace-less if-statements. I know it is bad but I love the simplicity of:
if (param == null) throw new ArgumentNullException("param");
I use var all over the f!#@ing place because I'm lazy as sin.
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 :)
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.
if(false){}
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.
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.
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!
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 :)
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.
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
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.
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.
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.
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.