|
Nish Sivakumar wrote: Sometimes, it is though.
not really - the comments that are there are irrelevant but the ones that are needed are missing.
The user is valid so why does it need updating?
|
|
|
|
|
// check if user is valid
if(IsUserValid(user))
{
// update the user
UpdateUser(user);
}
else
{
// show a messagebox with an error
MessageBox(error);
}
Worthless comments, agreed...but doesn't mean comments aren't expected.
I'd much rather have a comment at the beginning of the code segment saying what this chunk of work means, rather than what each step does...for example:
</twocentsworth>
I used to call it "Super Happy No-Pants Wonder Day"! It turns out that the police just call it "Tuesday". Go figure...
|
|
|
|
|
This is how I try to comment. If the code section is complex I'll put comments that refer to the overall comment just to help the reader track where in the logic we're running. For instance
/*
* There are several constraints that need to verified.
*
* Constraint 1:
* Constratin 2:
*
***
*/
In the code I'll put a comment to provide a quick reference to each constraint right before I start checking that constrain.
|
|
|
|
|
There is a saying that goes something like "you can tell how smart someone is by how much they agree with you". Kudos to every one of these bad coding habit comments! I can tell every one of you has not only developed, but MAINTAINED real life code!!!
I would like to make one more statement in favor of good comments. In the classical book by Fred Brooks (The Mythical Man Month), he stated "always throw the first version away". When I am about to code something non-trivial, I almost always write the comments first, as I consider that my "first version". I figure that if I cannot describe in words what I need to do, then I probably can't describe it in code either.
|
|
|
|
|
Normally happens if you write the comments before you write the code. It is just forgetting to remove the obvious comments after the code has been written.
|
|
|
|
|
I'd add :
6) Leaving commented out code in source control.
|
|
|
|
|
Since I don't do this for a living anymore, my opinion probably doesn't count for much. But back when I did do it, the cost of maintaining code far exceeded the cost of developing it, and I considered a lack of meaningful comments grounds for termination. I still do.
Others in my list would include leaving commented-out code in production source, and embedding numeric constants in code for use in calculations. I don't know if that last one is common anymore, but it used to drive me nuts, and I found it in a lot of code.
Will Rogers never met me.
|
|
|
|
|
Roger Wright wrote: I don't know if that last one is common anymore, but it used to drive me nuts, and I found it in a lot of code.
Oh, it's still common!
I found some code a while ago that the dev had obviously thought he'd done the right thing...
const int FiveHundred= 500;
Sure, re-factoring is easier (although it was only used in one place anyway) but not the most meaningfull names! (it was for a 1/2 second time out time)
PooperPig - Coming Soon
|
|
|
|
|
Well, he tried, as you say. I used to program automated test equipment for missile guidance systems, and each test station had to be initiated with a local gravity vector for its physical location. Some programmers simply hard-coded a three-valued constant into the code; DATA 0.00340120, 0.00002101, 32.16254301, or some such. Moving the machine to a new location meant recoding gravity at that point, but nothing in the code told what numbers were gravity. That's just one example, and there were many much worse.
Will Rogers never met me.
|
|
|
|
|
Reminded me of a story I heard about a dev who had written guidance software for tank aiming - the idea being the operator could identify a target and the software would move the barrel to track the object and fire when aimed.
The story goes that the first time it was tried out on a real tank, the tank fired almost immediately - in entirely the wrong direction. Turned out that the software was full of literal values, and had been fudged during testing so the devs didn't have to wait while a virtual barrel turned laboriously around - and they'd missed a value when they took out the changes in the real McCoy!
Not sure I believe it (as surely there'd need to be some feedback from the tank) but nice image of lots of brass and boffins ducking for cover!
PooperPig - Coming Soon
|
|
|
|
|
I don't have any trouble believing that story, as I've been part of worse. Back in the days when the Phalanx gun system was being developed by my company, at a demonstration session they fired it up in front of a bunch on Navy brass (unloaded, but still noisy as hell), it deployed from its clamshell mount, spun around and locked on the first available target - the bridge of the ship where all the brass stood watching. I suspect a lot of dress whites needed cleaning that afternoon, and I dearly wish I could have been there to see it. Happily, I was still in school when I started working at General Dynamics, and my Control Systems instructor was a part timer whose day job was chief engineer for Phalanx at General Dynamics. He was there, and told us all about it. Then he assigned us a 10 question take home, open book exam that contained all the field measurements, design equations for the control loop, and target classification algorithms, and required us to solve the problem and properly assign constraints and design compensation circuits to stabilize the control system. Apparently we managed to solve the problem for him, as the error never happened again, and I passed the class. That was the single, most difficult test I ever took, as it was all using real world data - not theoretical nonsense that textbooks present - and each question depended upon the answer to the one before it being correct - exactly how the real world works for an engineer. If you get step two wrong, in the real universe, everything that follows will also be wrong, though you might not notice that until step 28, years and million$ later. 
Will Rogers never met me.
|
|
|
|
|
Ah, the Helen Keller code! I did that once on a conveyor I was programming. Didn't want to make labels for the boxes and so I substituted the label read for a list read. My intent was to load the entire conveyor with boxes and then have all the diverters (it was a two sided sorter) fire at once to make a fireworks display.
Psychosis at 10
Film at 11
Those who do not remember the past, are doomed to repeat it.
Those who do not remember the past, cannot build upon it.
|
|
|
|
|
That's a worry considering what those systems did 
We can’t stop here, this is bat country - Hunter S Thompson RIP
|
|
|
|
|
Could have been worse, like
const int FiveHundred= 450;
GOTOs are a bit like wire coat hangers: they tend to breed in the darkness, such that where there once were few, eventually there are many, and the program's architecture collapses beneath them. (Fran Poretto)
|
|
|
|
|
Some code I write includes the "sneaky minus".
someFunc(300, 250 * abc, -(500-otherVar * (3+abc), 592.3f);
I generally document that, unless the logic shouldn't ever need changing.
|
|
|
|
|
SortaCore wrote: unless the logic shouldn't ever need changing
That's a rather roundabout way of saying 'always'
GOTOs are a bit like wire coat hangers: they tend to breed in the darkness, such that where there once were few, eventually there are many, and the program's architecture collapses beneath them. (Fran Poretto)
|
|
|
|
|
Yeah, when coordinates are no longer used in printing, the logic will need changing. 
|
|
|
|
|
>back when I did do it, the cost of maintaining code far exceeded the
>cost of developing it, and I considered a lack of meaningful comments
>grounds for termination. I still do.
+1
A friend of mine teaches programming for high-school students - and docks a student's grade if the programs aren't reasonably commented.
Back in the early 1960s while in college I made some changes to a copy of Spacewar, and still have the source listings...but I have no idea today what those changes were. I hadn't learned about commenting (and in general neither had the other programmers who worked on the code).
Now, as the senior engineer in my department at my POE I use those uncommented listings as examples of poor programming practice. (There are few things so horribly bad that they can't be used as a bad example.)
|
|
|
|
|
7. Having two different methods that do exactly the same thing but with the arguments in a different order. I have come across this at at least three different places I have worked. Which one to delete when refactoring?
- I would love to change the world, but they won’t give me the source code.
|
|
|
|
|
Forogar wrote: Which one to delete when refactoring?
Both.
GOTOs are a bit like wire coat hangers: they tend to breed in the darkness, such that where there once were few, eventually there are many, and the program's architecture collapses beneath them. (Fran Poretto)
|
|
|
|
|
I've encountered a similar, but annoying variant of this. There were a whole bunch of functions in my old employer's code base which did nothing but call an almost identically named function, such as:
someFunc(int a, string b)
{
return some_Func(a, b);
}
In some cases, this went on for a step or two further, with some_Func calling some__Func (I HATE double underscores in code), which then again finally called the 'real' function someOtherFunc.
I suppose it was the result of a messed up attempt at refactoring. It was hugely annoying when debugging.
|
|
|
|
|
Spoon Of Doom wrote: nothing but call an almost identically named function
I've seen that too. It was in just plain C, in some code that represented a layered architecture -- so it was similar to:
BL_GetDate() { return DAL_GetDate() ; }
An OOP version might be:
F() { base.F() ; } 
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
In order of how I have them listed below:
0) Use of VB.
1) Use of Convert and/or ToString rather than casting and/or Parsing.
2) Over-use of Reflection. Not caching and reusing information retrieved via Reflection.
3) Over-reliance on tools, especially third-party tools.
4) Monolithic classes, lack of modularity, non-single-responsibility.
5) Singletons. 
6) Convoluted concatenation -- a String.Format will be clearer.
6.1) Concatenated SQL statements, when a parameterized statement is better on so many levels.
7) Not leveraging interfaces.
8) Not allowing polymorphism for no apparent reason.
9) Swallowing Exceptions.
10) Posting snippets of code that use uncommon, custon, or third-party classes and expecting everyone to know what they are.
You'll never get very far if all you do is follow instructions.
modified 24-Jun-14 14:44pm.
|
|
|
|
|
PIEBALDconsult wrote: 5) Singletons.
I reckon valid use cases for singletons. Facade patterns, for example, especially in C++ code.
PIEBALDconsult wrote: 1) Use of Convert and/or ToString rather than casting and/or Parsing.
I lost you there, can you please clarify?
The console is a black place
|
|
|
|
|
PIEBALDconsult wrote: 6.1) Concatenated SQL statements
Revoke the programming license of anyone who does this.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
