|
I have seen people use ints with 1s and 0s because they learned programming with a language that did not have a boolean type. I say that is what you get when you have someone whose degree is not in programming write the software
What is really scary is when folk with a real programming degree do the same thing!
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I'm currently ROFL-ing inside, guys!!! A true pearl.
Notice, that ANY int suits the condition )))
|
|
|
|
|
I must have really been tired last night not to catch that. I'm having the same laugh now.
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
Indeed, that was the real bug I fixed and one of the two things that was odd about the code (excluding the fact that it's VB.Net rather than C#, which is obviously a flaw too).
|
|
|
|
|
aspdotnetdev wrote: VB.Net rather than C#
Very obviously...
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
Heh heh.. messed up boolean conditions are the bane of so many.. I think boolean logic is one of the hardest things to read well, and is one reason why I unit test my code thoroughly. Misused AND and OR is so easy to miss on code inspection.. but good tests never lie.
|
|
|
|
|
With that kind of range checking, the coder is more suited to checking the range where the burgers are being cooked.
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
|
|
|
|
|
Either that, or being brought to the range to be slaughtered. Which wouldn't be much different, because he'd end up at the burger place anyway.
|
|
|
|
|
Shhh!!! Don't give John any ideas!!!
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
I was digging around some code I wrote three years ago, looking for the cause of a bug some reported, when I found this. And no wonder - the full code is so full of holes that I'm surprised it works as expected.
private static bool CheckSpecialCases(List<List<int>> adjancies, Dictionary<int, Region> regions)
{
if (regions.Count < 2) return true;
if (regions.Count == 2)
{
int key1 = 0, key2 = 0, inc = 0;
foreach (KeyValuePair<int, Region> kvp in regions)
{
if (inc == 0)
{
inc++;
key1 = kvp.Key;
}
else
key2 = kvp.Key;
}
lock (listLock)
{
if (adjancies[key1].Contains(key2))
return true;
adjancies[key1].Add(key2);
adjancies[key2].Add(key1);
return true;
}
}
return false;
}
Hmm, where to start. How about that comment "True if it was a special case". Yes, because it is obvious what a special case is. Then the whole looping over a collection of two items, with a flag to set two items to specific values (instead of say, asking for the keys of the Dictionary). Oh, and the whole reason for the reported bug is that the indices of key1 and key2 were not checked for being in bounds somewhere prior in the code, which would have saved user trouble from this. Plus, the redundancy of using lists to specify connections, while matching the underlying implementation of data that I was working with, makes things unnecessarily complicated.
|
|
|
|
|
Harsh! I hope the guy or gal who wrote this can take a little constructive criticism.
|
|
|
|
|
jamie550 wrote: unnecessarily complicated
In OOP they call it "extendable".
Greetings - Jacek
|
|
|
|
|
A company hired me for cleaning up the PHP-code the CEO created. The whole code reads like a how-not-to of programming, so I decided to post some of the gems here. This is the first one (I preserved all the formatting):
MyCart::IsContentvalid() checks items in the cart against a database. Each call does 3 queries (per item!) of tables with around 500.000 entries. Note also the clever use of quotation marks.
if(MyCart::IsContentvalid()==1)
{
Helper::Redirect("confirmation.php");
}
if(MyCart::IsContentvalid()==2)
{
$this->setErrorMsg("$curlang[error_msg1]");
}
if(MyCart::IsContentvalid()==-12)
{
$this->setErrorMsg("These items can not be sold.");
}
if(MyCart::IsContentvalid()==15)
{
$this->setErrorMsg("Your account doesn't allow you to buy these items.");
}
else
{
$this->setErrorMsg("$lang[error_msg2]");
}
Oh, and the use of else is only for advanced programmers:
if ($result == 1)
{
}
if ($result != 1)
{
}
More will come soon!
|
|
|
|
|
I **really** like the second one.
|
|
|
|
|
Maybe it's just because its a Friday afternoon but I had to laugh at the following code I just found for removing the first 7 chars of a string....
s = s.Replace(s.Substring(0,7), "");
... See the funny side? or is it just me?
Life goes very fast. Tomorrow, today is already yesterday.
|
|
|
|
|
|
That's a particular funny way of doing it wrong yes
|
|
|
|
|
s = "badcoder you creazy badcode?"
s = s.Replace(s.Substring(0,7), "");
(it's 1 am here)
Greetings - Jacek
|
|
|
|
|
dont you mean...
Life goes very fast. Tomorrow, today is already yesterday.
|
|
|
|
|
bool hasWarnings = false;
if (!hasWarnings)
{
hasWarnings = true;
}
|
|
|
|
|
I just saw a piece of javascript today which effectively looked like this:
function check_somethingOrOther()
{
someVariable = false;
setTimeout("check_somethingOrOther()", 500);
} Now, I know that someVariable was obviously meant to do something, but this is the only place in the code that it's actually used. Optimising this code was really simple.
|
|
|
|
|
Yes, but what if it changes. You have to make sure it is really false!
|
|
|
|
|
I actually removed this one from a piece of code this week. And believe it or not, it was actually being called.
private bool IsValid()
{
return true;
}
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
If it were a polymorphic function that could be overridden by a derived class then it could make sense. However, in your context it does seem very strange.
|
|
|
|
|
Unfortunately, no. This class was never inherited. I honestly don't think the guy who wrote it originally even knew what that was.
I wasn't, now I am, then I won't be anymore.
|
|
|
|