|
a) the comment is inaccurate -- the worst kind
b) a comment on this statement is entirely unnecessary
c) as someone else mentioned, the statement it comments uses a magic number. Defining it as a constant might also provide the explanation that this comment fails to provide.
d) than/then confusion: one of my pet peeves.
|
|
|
|
|
I just fell off my chair when I reviewed some old code C# of a colleague 
DateTime dt1;
DateTime dt2;
if (dt1.ToString("s").CompareTo(dt2.ToString("s")) < 0)
{
}
Nothing else to mention...
|
|
|
|
|
I have seen some pretty bad ways to compare date, but that is surely one of the worst! 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Found this bit of VB/VBA code to be interesting! 
If (Len(Replace(colourStyle, AP_colourStyle, "")) = 0) then
For some reason it took me a little while to realize that it simply meant:
if (Trim$(colourStyle) = Trim$(AP_colourStyle)) then
I could probably leave out the trim$() calls, but I like to make sure there are no pesky extra surronding spaces.
-EM
|
|
|
|
|
Not quite the same. What if colourStyle already has a length of 0.
"You get that on the big jobs."
|
|
|
|
|
|
Best catch of the 2 so far! Here's my 5.
I hadn't noticed that particular morsel, and it is a possibility given the usage of these vars elsewhere in the code, and how the values get attributed to them.
Again fine catch. Kudos!
Hmm, does anyone really say "Kudos" anymore??????? 
-EM
|
|
|
|
|
Incorrect. Replace() replaces every instance, so with
colourStyle="APAPAP"
AP_colourStyle="AP"
the string would vanish and fire the original if, and not yours.
Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum
Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.
|
|
|
|
|
|
That's a great point and here's my 5.
These vars could never have that type of situation due to how they get their data, but the basic premise you point out is correct.
Assuming that the 2 vars refer to the same type of data (i.e. a "ColourStyle"), but from 2 different sources, I still don't see why someone would create that line of code. What design instructions would lead someone to use that type of logic? Any ideas, I'm interested to see other's take on this.
-EM
|
|
|
|
|
emartinho wrote: These vars could never have that type of situation due to how they get their
data
But in cases where your development and coding is outsourced (and maybe even internal), this is a bad premise in itself, since they will cut and paste so MUCH of your existing code base, the error these two variables 'will never encounter' will show up everywhere else in your code.
|
|
|
|
|
Both code goes to the category of Hall of Shame? 
|
|
|
|
|
For our remote embedded system, we have a convoluted patching mechanism. For our latest release, to solve another problem with large archives I rewrote our unzipper. It worked great except a critical file would get deleted. We threw more logging, changed the organzation of the zip files to no avail. Another developer added some logging which identified when the file in question was getting deleted and asked me what circumstances led to that error condition. I looked over his shoulder, my eyes went up a few lines and I saw it.
To take care of a very rare edge case, I added a check. Problem is that if the file being unzipped was exactly the same size as the unzip buffer, that check would erroneously fail. To make it worse, I realized that the check would have never failed (aside from the above) unless the zip file was deliberately corrupted, including adding fake CRCs and even then it would be very difficult. Point being, I added a test for an edge case that would never happen causing an edge case that did!
|
|
|
|
|
Found this in JavaScript code from one of our designers
if(a>b && a<=b)
{
}
else
{
}
|
|
|
|
|
Well I am sure a designer would read that as...
if a is looking at b while on the other side of the bushes b is entering a from behind
do something
else
do something else
ALTERNATIVELY
The designer knows not how to apply multi lines commenting?
I may or may not be responsible for my own actions
|
|
|
|
|
Yes, that is truly an impeccable bit of logic. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
No this is just a more esoteric way of commenting out code.
//do something is the stuff being commented out and
//do something else is now being done instead.
I think this is obvious, isn't it?
Cheers!
|
|
|
|
|
I dont believe your designer wanted it that way, but a comparison to a NAN is always false.
I am using a similar NAN-Check
if (x != x) ...
gets true if x is NAN
|
|
|
|
|
if(x.isNaN()) (Java)
if(double.IsNaN(x)) (C#)
modified on Wednesday, May 4, 2011 1:53 PM
|
|
|
|
|
|
the unequality test is more portable
i actually used it in CUDA, afterwards i found out there exists an isnan(x) builtin function
now porting to other languages is more easy
|
|
|
|
|
Then surely one will want to do:
if(a > b && a <= b && a != b && !isNaN(a + b)) { }
|
|
|
|
|
I think I just threw up a little in my mouth.
|
|
|
|
|
laugh all you wish, but that's not to far fetched from something some of the tiddlywinks I had classes with would do. Might be why I have a job and they don't but, that's another issue...
Programming is a race between programmers trying to build bigger and better idiot proof programs, and the universe trying to build bigger and better idiots, so far... the universe is winning.
|
|
|
|
|
//do something else; like working as an accountant maybe!
"You get that on the big jobs."
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
