Using while... break as an if...else replacement.
-
I just found this code.
while(1)
{
// 15 lines of codeif (count == 0)
{
LogErrorMessage();
break;
}
// 30 lines of code
break;
}Instead of something like this
// 15 lines of code
if (count == 0)
{
LogErrorMessage();
}
else
{
// 30 lines of code
}There were no other breaks or continues, and if there were lots of reasons to break it should be in a separate function anyway otherwise you're basically using a glorified goto statement.
-
Well, at least they saved -1 indent levels...
-
It would be really neat if there was a continue statement somewhere in there.
-
@Ben L. said:
Well, at least they saved -1 indent levels...
-
It looks like it was supposed to be a loop but it kept blowing up, so they added the LogErrorMessage and break, and made it only run once. Probably.
Out of curiosity, what did LogErrorMessage do? I'm guessing it probably didn't log a useful error message...
-
@anotherusername said:
Out of curiosity, what did LogErrorMessage do? I'm guessing it probably didn't log a useful error message...
The function was commented because the program kept reporting errors. Now the program doesn't report any errors, which is much better.
-
@Kian said:
@anotherusername said:
I was more interested in the utter and complete lack of useful information being passed to indicate WHAT error happened or WHERE.Out of curiosity, what did LogErrorMessage do? I'm guessing it probably didn't log a useful error message...
The function was commented because the program kept reporting errors. Now the program doesn't report any errors, which is much better.
-
@anotherusername said:
@Kian said:
@anotherusername said:
I was more interested in the utter and complete lack of useful information being passed to indicate WHAT error happened or WHERE.Out of curiosity, what did LogErrorMessage do? I'm guessing it probably didn't log a useful error message...
The function was commented because the program kept reporting errors. Now the program doesn't report any errors, which is much better.
or they also logged the stack trace so only the programmer would know what the error was...
-
@anotherusername said:
It looks like it was supposed to be a loop but it kept blowing up, so they added the LogErrorMessage and break, and made it only run once.
Either that, or they were trying to use "do-while-false" (to avoid using "goto", as suggested above) but got confused at some point.
-
@anotherusername said:
Out of curiosity, what did LogErrorMessage do? I'm guessing it probably didn't log a useful error message...
I removed the parameters being sent to that function for anonymisation purposes, I didn't consider it particularly essential to this wtf. I would have added something like "there was an error message here" but last time I did that someone compained about the code not fitting on their (presumably) 800px wide screen.
As far as I can tell there has never been a reason for the code to loop so I'm still thinking it's a dodgy attempt at a goto. Many functions in this part of the product used to take 80+ parameters before we refactored it to use some objects, this made it very difficult to put anything into separate functions leading to some 10000+ line long functions. There's one function where the first 500 lines are purely local variable declarations.
-
@Mithious said:
I removed the parameters being sent to that function for anonymisation purposes, I didn't consider it particularly essential to this wtf. I would have added something like "there was an error message here" but last time I did that someone compained about the code not fitting on their (presumably) 800px wide screen.
It's too bad there isn't a short punctuation mark that you could use...
-
@anotherusername said:
@Mithious said:
Using one three times is a bit repetitive, there is an alternative…I removed the parameters being sent to that function for anonymisation purposes, I didn't consider it particularly essential to this wtf. I would have added something like "there was an error message here" but last time I did that someone compained about the code not fitting on their (presumably) 800px wide screen.
It's too bad there isn't a short punctuation mark that you could use...
-
@PJH said:
@anotherusername said:
Oh, I've been trying to cut back on the Unicode. Those multi-byte encodings really stick to the hips.@Mithious said:
Using one three times is a bit repetitive, there is an alternative…I removed the parameters being sent to that function for anonymisation purposes, I didn't consider it particularly essential to this wtf. I would have added something like "there was an error message here" but last time I did that someone compained about the code not fitting on their (presumably) 800px wide screen.
It's too bad there isn't a short punctuation mark that you could use...