|
there should be minimum comments in the code, as they are not maintained properly and often lead to confusions.
comments should be put unless some very convoluted things are going on
No matter how fast light travels, it finds the darkness has always got there first, and is waiting for it
|
|
|
|
|
And not to mention, in most cases comments a) states the obvious or b) lie to you, and only function as code-clutter. comments are the most misused feature of any programming language.
|
|
|
|
|
Another sample from our legacy code base...
If someVar = "2" = True Then
|
|
|
|
|
Its scary to think that someone who graduated with any sort of programming degree would write that.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Obviously, the culprit misheard the comment "Too true!"
Peter
Software rusts. Simon Stephenson, ca 1994.
|
|
|
|
|
I'm tasked, as many of us are, with maintaining a codebase that was written by my predecessor. I'm finding some parts disgusting, and other, more like... fun?
Like the use of 'ToLower()' to make string comparison more safe...
if (someString.ToLower().StartsWith("text", StringComparison.OrdinalIgnoreCase))
-> Nice way to waste some cycles and memory for the Garbage Collector to play with!
if (someString.Substring(lastMarkend - 3, 3).ToLower() == "<Index>")
-> My little finger tells me, the functionality under this if must have been tested quite thoroughly.
NB: I've only changed the string names.
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
Oh oh! I've got the answer: #2 is false
|
|
|
|
|
#1, if you start counting at the first number.
|
|
|
|
|
Shouldn't #2 be:
if (someString.Substring(lastMarkend - 3, 3).ToLower() == ("<Index>").ToLower())
|
|
|
|
|
That would work better for sure
BTW, the string literal is just that, a string literal, when there are several 'consts'* defined some lines above
* without any const specifier or special naming of course!
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
#2 is even worse. The literal on the right is 7 characters long, the string on the left only 3.
|
|
|
|
|
Well spotted, I did stop at the case mismatch there!
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
I discovered this little gem courtesy of our lead developer.
Application.DoEvents();
Thread.Sleep(1000);
Application.DoEvents();
This is in a loop on the UI thread, called from a forms timer.
|
|
|
|
|
|
Application.DoEvents and Thread.Sleep are a classic combo that I have seen far too many times.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Coded by lazy programmers, for lazy programmers... This is what happens when you sleep at work!
It's an OO world.
|
|
|
|
|
Just read this story in the lounge[^]. Reminds me of a true shame...
Our 'senior', SQL expert recently wrote some dynamic SQL SP recently.
He is fully aware of SQL injection and actually went about it keeping security in the back of his head.
This is what he came up with (just his idea, not the query)...
sql_statement = 'select '''@param''' from someTable'
I took a look at the query and asked him what in the hell all those apostrophes where doing there (the thing was full of it, lots of magic going on there!).
He said that was to prevent SQL injection. Can you imagine my reaction?
I broke it with a single search for "D'artagnan"
It was then and there that I (a SQL rookie) thaught him about quotename[^]...
It's an OO world.
modified on Tuesday, August 30, 2011 10:23 AM
|
|
|
|
|
Now can you get someone to teach you both about bound parameters
|
|
|
|
|
Very true indeed! I was not aware of them (or perhaps I am, but not of the name). However, I'm not the SQL expert
It's an OO world.
|
|
|
|
|
Parameterized queries are safe in this context!
Well there are those 'Xperts' and 'Seniors! everywhere Try this[^]
|
|
|
|
|
I don't know - it isn't in my code anymore...
I wrote this wonder this morning, and realized I could prune it down to just the one line:
private void SetRole(SMUser user, UserRole userRole, string roleName, List<string> inRoles, List<string> outRoles)
{
(((user.Roles & userRole) != 0) ? inRoles : outRoles).Add(roleName);
}
I mean, yes it works. But...
It's not there anymore: I changed the whole way I handle roles for other reasons. But still, It's a bit impenetrable...
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
In my opinion, it's a nice piece of code, you should take it to the 'Clever code' forum. Readability is a problem though.
|
|
|
|
|
It's the readability I don't like at all - that is not clever, it's just downright nasty!
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
I wouldn't qualify it as horror. It's just a little bit too clever.
With a good comment to explain it quickly, or a more explicit method name, it would be fine by me.
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
I have to agree its not really horrible; as long as it is well commented it is ok.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|