Using while... break as an if...else replacement.



  • I just found this code.

    while(1)
    {
    // 15 lines of code
    	if (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:

    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.

    I was more interested in the utter and complete lack of useful information being passed to indicate WHAT error happened or WHERE.


  • @anotherusername said:

    @Kian said:
    @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.

    I was more interested in the utter and complete lack of useful information being passed to indicate WHAT error happened or WHERE.
     

     

    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...


  • Discourse touched me in a no-no place

    @anotherusername said:

    @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...
    Using one three times is a bit repetitive, there is an alternative…



  • @PJH said:

    @anotherusername said:
    @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...
    Using one three times is a bit repetitive, there is an alternative…
    Oh, I've been trying to cut back on the Unicode. Those multi-byte encodings really stick to the hips.


Log in to reply