Masquerading as a Loop

« Return to Article
  • Larry 2010-08-25 11:00
    TRWTF is another language without a while loop.
  • ais523 2010-08-25 11:02
    I've actually done this with a do{}while(0) loop in C, because it was clearer than the alternatives. (This was generated code, btw; the generated version was meant to be readable, but the original was the bit you actually edited.) It's a nice paradigm for what would otherwise lead to 20 nested if statements.
  • CodeRage 2010-08-25 11:05
    If you want that kind of jumpy behavior, refactor into a method and use returns;

    ...
    bool bCreateModel = shouldCreateModel();
    ...
    private boolean shouldCreateModel()
    {
    if (!pModel)
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    return true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    return true;
    }
    }

    return false;
    }
  • by 2010-08-25 11:06
    The "break;" as the last statement in the loop is like the punchline at the end of a joke.
  • Crash Magnet 2010-08-25 11:08
    Another example of "Stupid Language Tricks".
  • Wrongfellow 2010-08-25 11:08
    I've seen this kind of construct a few times. The worst one consisted of three nested "loops" and spanned about seven pages of code.

    Usually these things are produced by people who've read that "GOTO makes code unreadable" and started mindlessly parroting it without really understanding it.

    But in this case TRWTF is that it never loops at all and can easily be rewritten as an "if(...) { ... } else if(...) { ... }" block without any breaks or faux-loops at all.

    (Of course, maybe this code was written for an embedded system which doesn't have an "else" keyword.)
  • A Tester 2010-08-25 11:09
    If you want that kind of jumpy behavior, refactor into a method and use returns;

    ...
    bool bCreateModel = shouldCreateModel();
    ...
    private boolean shouldCreateModel()
    {
    if (!pModel)
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    return true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    return true;
    }
    }

    return false;
    }

    but that wouldn't reproduce the original behaviour. Try
    private boolean shouldCreateModel()
    {
    if (!pModel)
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    return true;
    }

    boolean retval = false;
    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    retval = true;
    }
    }

    return retval
    }

    you have to make sure you complete the inner loop, not jump out of it early
  • The Nerve 2010-08-25 11:11
    bool bCreateModel = false;
    
    if (!pModel || asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize() && !bCreateModel; ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    }
    }

    Fixed?
  • Michael Cox 2010-08-25 11:12
    This looks like C++. Why not just use the goto keyword?
  • transverbero 2010-08-25 11:15
    The real WTF is you and the people responsible for firing this guy. He is clearly the only one who knows his job.

    This is the best one can do without using a goto which you probably don't even have in whatever bastard language that is.
  • Alex 2010-08-25 11:16
    No. Now the for loop that goes through all the slow operations of checking each part gets run each time.
  • The Nerve 2010-08-25 11:17
    CodeRage:
    If you want that kind of jumpy behavior, refactor into a method and use returns;

    ...
    bool bCreateModel = shouldCreateModel();
    ...
    private boolean shouldCreateModel()
    {
    if (!pModel) // global variable???
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize()) // global variables???
    {
    return true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    return true;
    }
    }

    return false;
    }
  • toth 2010-08-25 11:17
    the article:

    Thankfully, the fellow responsible is 'no longer with us'


    I hope that means he was executed.
  • Cbuttius 2010-08-25 11:19
    With regards to the code I would prefer to:

    - Get all that code into a function and return true or false, not break with some variable set.

    - Don't use Hungarian notation

    - Use a standard C++ container. They don't have methods called GetSize()

    - Use std::equal for the later loop rather than a handwritten loop.

    something like this:

    bool ModelHolder::mustCreateModel() const
    {
    return !model
    || modelPartsToLoad.size() != modelParts.size()
    || !std::equal( modelPartsToLoad.begin(), modelPartsToLoad.end(), modelParts.begin() );
    }

  • Carl T 2010-08-25 11:22
    A Tester:
    If you want that kind of jumpy behavior, refactor into a method and use returns; [snip]

    but that wouldn't reproduce the original behaviour. Try
    [snip]
    you have to make sure you complete the inner loop, not jump out of it early

    Did you not see the break; in the inner loop in the original code?
  • Herby 2010-08-25 11:24
    Other than having an infinite loop
    for (;;)
    I see little wrong. As others have suggested, a better looping structure for this type of construct is:
    do { ... } while (0);

    Yes sometimes
    goto
    is evil, but this is a nice way of doing things if you have been told not to use them. for the others who insist, Fortran 66 is available in some compatibility mode, be my guest.
  • The Nerve 2010-08-25 11:24
    Alex:
    No. Now the for loop that goes through all the slow operations of checking each part gets run each time.


    Why? Is it for loops or boolean operators that work differently in C++ than they do in Java?

    The Nerve:

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize() && !bCreateModel; ++i)

  • Mad Adder 2010-08-25 11:26
    A Tester:
    If you want that kind of jumpy behavior, refactor into a method and use returns;

    ...
    bool bCreateModel = shouldCreateModel();
    ...
    private boolean shouldCreateModel()
    {
    if (!pModel)
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    return true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    return true;
    }
    }

    return false;
    }

    but that wouldn't reproduce the original behaviour. Try
    private boolean shouldCreateModel()
    {
    if (!pModel)
    {
    return true;
    }

    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    return true;
    }

    boolean retval = false;
    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    retval = true;
    }
    }

    return retval
    }

    you have to make sure you complete the inner loop, not jump out of it early


    Why complete the inner loop? The CodeSOD breaks out of the inner loop as soon as it sets the Boolean to true, and A Tester does the same thing by returning true when the condition is met. In fact, your code completes the loop unnecessarily. As soon as you set the return value to true, each iteration afterwards is a no-op. Either the condition is false and the return value stays the same, or the condition is true and you set an already-true variable to true. In the end, it accomplishes the same thing with potentially more steps.
  • Bert Glanstron 2010-08-25 11:27
    toth:
    the article:

    Thankfully, the fellow responsible is 'no longer with us'


    I hope that means he was executed.


    This code is exactly what’s wrong with C++! He is dragging down the whole system with his idiocy! I insist that his fingers are cut from his hands completely!
  • JJ 2010-08-25 11:31
    For everyone who has had the "GoTo is Evil" Kool-Aid forced down their throats, I'll repeat my mantra: "No language feature is evil. It is the misuse of a language feature that is evil."
  • The Nerve 2010-08-25 11:34
    JJ:
    For everyone who has had the "GoTo is Evil" Kool-Aid forced down their throats, I'll repeat my mantra: "No language feature is evil. It is the misuse of a language feature that is evil."

    Or maybe sometimes the legitimate use? I submit the following for consideration.
    HashMap<String, ArrayList<String>> map = new HashMap<String, ArrayList<String>>();

    (Yes, I know this doesn't work in C++, but somehow the Java parser knows how to handle the >>)
  • Ken B. 2010-08-25 11:34
    by:
    The "break;" as the last statement in the loop is like the punchline at the end of a joke.
    Without it, you would have an infinite loop if the first two conditions were false.

    On the other hand, the original coder apparently never heard of "else".
  • Cbuttius 2010-08-25 11:35
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?


    You're fired
  • Edward von Emacs, VI 2010-08-25 11:35

    loop:
    for (post = first; post.name == "Darth*" || post.name == "*Gland*"; post = post.next)
    goto head_esplode;

    head_esplode: head_esplode();
    goto loop;


    You'd think your head could only esplode *once* per Darth GlandStorm post. Not so. In the words of Edsgar Dijkstra, "Two or more, use a hammer." Or something like that.
  • Anonymous 2010-08-25 11:37
    JJ:
    For everyone who has had the "GoTo is Evil" Kool-Aid forced down their throats, I'll repeat my mantra: "No language feature is evil. It is the misuse of a language feature that is evil."

    Right, just like "guns don't kill people, people kill people". Of course, a having a tool that is specifically designed for killing people certainly makes the person's job a lot easier. But yeah, guns don't kill people and GOTO is a perfectly valid high-level language construct.
  • plaga 2010-08-25 11:37
    by:
    The "break;" as the last statement in the loop is like the punchline at the end of a joke.

    Yes it certainly makes me want to punch the idiot who wrote that mess!
  • Andy P 2010-08-25 11:38
    I came across a construct absolutely identical to this today.

    On a whim, I hunted through the codebase for more examples - and found plenty. All written by people still at the company, in fact, three out of the four people on my team (the fourth being me).

    DailyWTF CodeSOD accurately describes my daily work? FML. :-(
  • NASA Engineer 2010-08-25 11:38
    No, you guys don't get it. This loop works like the "slingshot" maneuver that probes do to pick up speed before leaving orbit. Using a loop and then breaking out of it actually makes the code run faster!
  • Edward von Emacs, VI 2010-08-25 11:38
    Edward von Emacs, VI:

    loop:
    for (post = first; post.name == "Darth*" || post.name == "*Gland*"; post = post.next)
    goto head_esplode;

    head_esplode: head_esplode();
    goto loop;



    Maybe that should be something like `matches(post.name, "Darth*") ...'

    Just making this stuff up as I go.
  • Cbuttius 2010-08-25 11:38
    goto is entirely inappropriate here though, just a few || operators.
  • subanark 2010-08-25 11:44
    I guess C doesn't have labeled breaks? E.g.

    Label:
    {
    ...
    break Label;
    ...
    }
  • danixdefcon5 2010-08-25 11:47
    Edward von Emacs, VI:
    Edward von Emacs, VI:

    loop:
    for (post = first; post.name == "Darth*" || post.name == "*Gland*"; post = post.next)
    goto head_esplode;

    head_esplode: head_esplode();
    goto loop;



    Maybe that should be something like `matches(post.name, "Darth*") ...'

    Just making this stuff up as I go.


    Your code was successful. My head exploded after reading it. ;)
  • vydf 2010-08-25 11:49
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?


    We found a witch, may we burn him?
  • by 2010-08-25 11:50
    vydf:
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?


    We found a witch, may we burn him?

    I'm not really comfortable with you burning Mike Cox.
  • Maurits 2010-08-25 11:52
    Meh. for (;;) { ... break; } is equivalent to do { ... } while(0);, which is one of three common memes in the code base I work in.

    MEME 1, goto:

    hr = GetResource(&r);
    if (FAILED(hr)) { goto Cleanup; }
    ...

    Cleanup:
    FreeResource(r);
    return hr;


    MEME 2, do { ... } while(0);

    do {
    hr = GetResource(&r);
    if (FAILED(hr)) { break; }
    ...
    } while(0);

    FreeResource(r);
    return hr;


    MEME 3, RAII:

    SmartResourceWrapperObject r;
    hr = GetResource(&r);
    if (FAILED(hr)) { return hr; }
    ...
    (FreeResource called in ~SmartResourceWrapperObject)


    As far as I can tell, none of these is inherently superior to the others.
  • TheCPUWizard 2010-08-25 11:54
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!

  • PsyberS 2010-08-25 11:55
    Someone needs to give that fellow a copy of the famous paper "For Loop Considered Harmful: A Retrospective".
  • TheCPUWizard 2010-08-25 12:00
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!

  • Cbuttius 2010-08-25 12:01
    The real WTF is that if the arrays are not the same size or don't match it should return true, and if they match it should return false, but if the pointer is NULL it should return fileNotFound.
  • pitchingchris 2010-08-25 12:03
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.



    From what I'm reading, it was intended to be an april fools joke and never got removed.
  • Cbuttius 2010-08-25 12:05
    comefrom would be very useful in C++, especially when catching an exception.
  • jdev 2010-08-25 12:08
    What is it with the weird for loop abusing lately?
    Whatever happened to the while loop when you are not iterating over an array or collection but you need to loop anyway?

    And to think some people actually blame java itself for this kind of crap code.
  • frits 2010-08-25 12:08
    Cbuttius:
    comefrom would be very useful in C++, especially when catching an exception.


    Sounds like you need to implement a stack trace.
  • dave 2010-08-25 12:19
    Java has the nice, largely unused feature of labeled blocks, which can be used in these cases (when you're too lazy extract the logic to another method).

    determineCreate: {
    if (!pModel)
    {
    bCreateModel = true;
    break determineCreate;
    }
    ...
    }
  • wtf 2010-08-25 12:20
    NASA Engineer:
    No, you guys don't get it. This loop works like the "slingshot" maneuver that probes do to pick up speed before leaving orbit. Using a loop and then breaking out of it actually makes the code run faster!


    Lovely. I'm stealing this for future use!
  • FuBar 2010-08-25 12:24
    by:
    vydf:
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?
    We found a witch, may we burn him?

    I'm not really comfortable with your burning Mike Cox.

    FTFY
  • Fred 2010-08-25 12:25
    NASA Engineer:
    No, you guys don't get it. This loop works like the "slingshot" maneuver that probes do to pick up speed before leaving orbit. Using a loop and then breaking out of it actually makes the code run faster!
    You have to give the compiler something to optimize away if you really want the maximum possible performance.
  • Moshe 2010-08-25 12:28
    This is actually a pretty common coding convention. It is not an elaborate GOTO but a poor man exception handling.

    It is usually used when you have many initializations intermixed with things that can fail and you want to be able to write all the cleanup code in one place (which was NOT the situation in the example).

    It is better written with a "do {} while (0);".

    This is not my style of coding, and in the specific code example it was not warranted at all. Yet this is a valid style and certainly not deserving a WTF.
  • newfweiler 2010-08-25 12:32
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!



    I remember the proposal to add COMEFROM, COMEFROM DEPENDING ON, (computed COMEFROM) and conditional COMEFROM to COBOL and ForTran.

    The modern language that has COMEFROM is Java, only it's called "Annotations". It's really a COMESUB rather than a COMEFROM, and duplicate labels are allowed.

    It's useful for application platforms where you want to call your own method from platform code, without modifying the platform code to add a line to call your method.

    For example:


    public class CustomerService {
    public Response getCustomer(String id) {
    COMEFROM (HttpRequestProcessor)
    WHEN (method.equals("GET")
    && path.startsWith("/app/customer");
    // construct response here
    Response resp = ...;
    GOBACK(resp);
    }
    }



    Actual code:


    @Path("/app/customer/{id}")
    public class CustomerService {
    @GET
    public Response getCustomer(@PathParam("id") String id) {
    // Construct response here
    Response resp = ...;
    return (resp);
    }
    }

  • newfweiler 2010-08-25 12:33
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!



    I remember the proposal to add COMEFROM, COMEFROM DEPENDING ON, (computed COMEFROM) and conditional COMEFROM to COBOL and ForTran.

    The modern language that has COMEFROM is Java, only it's called "Annotations". It's really a COMESUB rather than a COMEFROM, and duplicate labels are allowed.

    It's useful for application platforms where you want to call your own method from platform code, without modifying the platform code to add a line to call your method.

    For example:


    public class CustomerService {
    public Response getCustomer(String id) {
    COMEFROM (HttpRequestProcessor)
    WHEN (method.equals("GET")
    && path.startsWith("/app/customer");
    // construct response here
    Response resp = ...;
    GOBACK(resp);
    }
    }



    Actual code:


    @Path("/app/customer/{id}")
    public class CustomerService {
    @GET
    public Response getCustomer(@PathParam("id") String id) {
    // Construct response here
    Response resp = ...;
    return (resp);
    }
    }

  • lolwtf 2010-08-25 12:33
    "Thankfully, the fellow responsible is 'no longer with us'"

    Is that because you stabbed him?
  • JB 2010-08-25 12:41
    Maybe he didn't know about the "else" keyword...
  • Rick 2010-08-25 12:43
    pitchingchris:
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.



    From what I'm reading, it was intended to be an april fools joke and never got removed.

    The Pick BASIC language has a 'RETURN TO' statement, which was not implemented as an April Fools joke. The reader of a Pick BASIC program could not assume that execution would return to the next statement after a GOSUB.
  • Jay 2010-08-25 12:46
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?


    Because GOTO is considered harmful, of course. Everyone knows you shouldn't use a GOTO in a modern program.

    That's why people who don't understand structured programming have to simulate GOTOs with bizarre and twisted applications of other statements.

    Seriously, though, this is my problem with people who insist on following a dogmatic rule without bothering to udnerstand why the rule was invented in the first place.

    Hierarchy of good code:

    Good: Clean structured code
    Bad: GOTOs
    Worse: Using other statements to simulate GOTOs and thus making the program even more difficult to understand than if you had just used a GOTO.
  • Mark 2010-08-25 12:49
    I'm surprised how many people think the solution is to use multiple RETURNs. If you're really following structured programming rules (which is the only reason I can think you'd want to avoid GOTO), then your functions should have one unconditional RETURN at the very end.

    Dogma aside, function calls are more expensive than entering/exiting a loop. Maybe your compiler will fix that for you, but it won't fix that you're forcing someone reading the code to jump from place to place as they read in order to see what's going on. More functions isn't always better.

    Chains of conditionals are a tough problem. I've tried various solutions, depending on the language, and I generally don't like the options.

    If you really believe that sequences, conditions, and loops are the only things there are, then you can nest IF/ELSE after IF/ELSE... and I hope you have a nice, wide display terminal.

    I've toyed with

    if( StatementA
    && StatementB
    && StatementC
    )
    {
    }

    but that's a bit of an abuse of short-circuit logic.

    IMO the only sane solution is to use named blocks if your language lets you BREAK from named blocks - after all, BREAK is nothing but a structured-programming compromise on the GOTO issue anyway; or to give up and use GOTO if your language won't let you use a break.

    I don't like using a loop as a named block just to get a BREAK, and I'm not sure this sequence of conditionals is long enough to require that kind of solution anyway, but it hardly seems WTF-worthy.
  • Stuck in the Tarpit 2010-08-25 12:51
    In January I inherited code from someone who just left. It has a for-break loop like this that is 600 lines long. 600 lines of check-a-condition, set-an-error-variable and break.

    It made me twitchy just looking at it. I brought it up in the weekly team meeting and everyone 10 years younger than me said, "No, we like those." Now I feel twitchy and stabby (and get-off-my-lawny).

    Everyone accepts that it's wrong to have a function with multiple return points. Many code-scouring tools identify that as a problem. So, isn't multiple breaks within a loop wrong for the same reason? I'd much rather see:

    bool success = true;

    // Take 600 lines of crap and put it into separate condition-checker functions ()
    if (success)
    success = checkConditionA ();

    if (success)
    success = checkConditionB ();
    etc..

    Thanks for listening to my rant. No one here will listen...

    Captcha: Saepius - and they said we couldn't create a human-Octopus hybrid!
  • Roberto 2010-08-25 12:52
    As a matter of facts, "Code Complete" even suggest this trick.
  • Ryan 2010-08-25 12:52
    Couldn't a switch(true) have accomplished the same thing?
  • ZPedro 2010-08-25 13:07
    You know, I've done that kind of stuff… in a do { } while(0); fortunately, and for a use case more complex than that. It's useful in C code in case you have multiple temporary objects/resources allocated in the scope, that you need to clean up, as then you cannot easily refactor to put the "loop" in its own function and replace the breaks by a function return. I feel it's a restricted case of goto that's clean, as all jump at the same point, without any possibility for a labyrinth of gotos.
  • averageGuy 2010-08-25 13:08
    The Nerve:
    bool bCreateModel = false;
    
    if (!pModel || asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize() && !bCreateModel; ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    }
    }

    Fixed?


    Almost, but not quite.

    Your code would execute the for loop when the first condition was true. This would at least be wasteful and might lead to errors if !pModel meant that asModelPartsToLoad was not initialized.

    Putting an the for loop into an else clause would fix that.
  • frits 2010-08-25 13:09
    Mark:
    I'm surprised how many people think the solution is to use multiple RETURNs. If you're really following structured programming rules (which is the only reason I can think you'd want to avoid GOTO), then your functions should have one unconditional RETURN at the very end.

    Dogma aside, function calls are more expensive than entering/exiting a loop. Maybe your compiler will fix that for you, but it won't fix that you're forcing someone reading the code to jump from place to place as they read in order to see what's going on. More functions isn't always better.

    Chains of conditionals are a tough problem. I've tried various solutions, depending on the language, and I generally don't like the options.

    If you really believe that sequences, conditions, and loops are the only things there are, then you can nest IF/ELSE after IF/ELSE... and I hope you have a nice, wide display terminal.

    I've toyed with

    if( StatementA
    && StatementB
    && StatementC
    )
    {
    }

    but that's a bit of an abuse of short-circuit logic.

    IMO the only sane solution is to use named blocks if your language lets you BREAK from named blocks - after all, BREAK is nothing but a structured-programming compromise on the GOTO issue anyway; or to give up and use GOTO if your language won't let you use a break.

    I don't like using a loop as a named block just to get a BREAK, and I'm not sure this sequence of conditionals is long enough to require that kind of solution anyway, but it hardly seems WTF-worthy.


    What's wrong with this?

    int loadSize = asModelPartsToLoad.GetSize();

    bool bCreateModel =(asModelParts.GetSize() != loadSize ||
    !pModel);

    if(!bCreateModel )
    {
    for (UINT32 i = 0; i < loadSize; ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    }
    }
    }

  • helix 2010-08-25 13:18
    NOT a WTF, this is common for Embedded systems on small 8bit micro
  • mh 2010-08-25 13:22
    Moshe:
    This is actually a pretty common coding convention. It is not an elaborate GOTO but a poor man exception handling.

    And there I was thinking that exception handling was nothing but an elaborate goto to begin with...
  • Ken B. 2010-08-25 13:43
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.
    I believe you are referring to INTERCAL?
  • Kruiser 2010-08-25 13:48
    In C code, this is better than a case statement, especially for string operands, which are not supported.
  • Ken B. 2010-08-25 13:51
    Kruiser:
    In C code, this is better than a case statement, especially for string operands, which are not supported.
    Even better would have been "else".
  • IrishGuy 2010-08-25 13:58
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!



    As Ken B. mentioned it is INTERCAL.
    Although to be fair that language was designed to be as obtuse and evil as possible.

    For real world evil check out COBOL's "ALTER" statement.
  • Kruiser 2010-08-25 14:02
    If you use else (say 12 times, with some else if's) and later remove one condition; it gets complicated.
  • Larry 2010-08-25 14:06
    Kruiser:
    If you use else (say 12 times, with some else if's) and later remove one condition; it gets complicated.

    TRWTF is that you don't know how to use Perl:

    s/else if .*\{.*}//
  • fool 2010-08-25 14:19
    Never heard of "else" ??
  • Xamuel 2010-08-25 14:44
    Not seeing any real WTF here. The guy's code mirrors how he thinks about the process. To be honest, it's more readable than a lot of the alternatives offered up in the comments here.
  • EngleBart 2010-08-25 14:47
    I skipped over all of the meme battles and trolling after the first 30 or so posts, so I do not know if anyone proposed this replacement yet. It eliminates the break at the end of the for loop. Forgetting that break is very dangerous as some other poster pointed out.

    This is clearly a better way to perform the same task. You could even create optional, self-documenting constants shown in angle brackets. Of course make sure that none of the constants are 0!

    bool bCreateModel = false;
    switch (0)
    {
    default:
    <case SEE_IF_IT_WAS_EVER_LOADED:>
    if (!pModel)
    {
    bCreateModel = true;
    break;
    }

    <case SEE_IF_THE_SIZES_ARE_THE_SAME:>
    if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    break;
    }

    <case SAME_SIZE_BUT_ARE_THE_COMPONENTS_THE_SAME:>
    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    break;
    }
    }
    <case IF_I_GOT_HERE_I_DO_NOT_NEED_TO_DO_ANYTHING:>
    }
  • bah 2010-08-25 14:51
    I "I X'd your Y, pray I don't X any further"'d your post, pray I don't "I X'd your Y, pray I don't X any further" any further

    But seriously, in university, we were told that multiple returns from a function were okay. Not amazingly great, but better than the alternative of setting some variable, holding it above the water until the end and returning it. Unless it made more sense to do that.

    We weren't taught hard and fast rules on stuff like this, but we were encouraged to notice code smell, given on the complexity and natural form of the bit of code we were working on.
  • zzo38 2010-08-25 15:02
    Michael Cox:
    This looks like C++. Why not just use the goto keyword?
    There are some good reasons for using goto command, but in this case just making a function that can return true or false, is probably better.
  • v.dog 2010-08-25 15:10
    I found the problem- this line was meant to be a comment:

    for (;;)

    The code is designed to make you cry.
  • zzo38 2010-08-25 15:22
    EngleBart:
    I skipped over all of the meme battles and trolling after the first 30 or so posts, so I do not know if anyone proposed this replacement yet. It eliminates the break at the end of the for loop. Forgetting that break is very dangerous as some other poster pointed out.

    This is clearly a better way to perform the same task. You could even create optional, self-documenting constants shown in angle brackets. Of course make sure that none of the constants are 0!
    I don't like that, please!

    Maybe this way is better (although, this way uses goto, so if you want to use functions that return true/false you would do it differently):


    @ @<Check for create model@>= {
    bCreateModel=true;
    @<See if it was ever loaded@>;
    @<See if the sizes are the same@>;
    @<Same size, but are the components the same?@>;
    bCreateModel=false;
    yes_create_model: ;
    }

    @ @<See if it was ever loaded@>= {
    if(!pModel) goto yes_create_model;
    }

    @ @<See if the sizes are the same@>= {
    if(asModelParts.GetSize() != asModelPartsToLoad.GetSize()) goto yes_create_model;
    }

    @
    @s UINT32 int
    @<Same size, but are the components the same?@>= {
    UINT32 i=asModelPartsToLoad.GetSize();
    while(i--) {
    if (asModelPartsToLoad[i] != asModelParts[i]) goto yes_create_model;
    }
    }


  • zzo38 2010-08-25 15:57
    Rick:
    The Pick BASIC language has a 'RETURN TO' statement, which was not implemented as an April Fools joke. The reader of a Pick BASIC program could not assume that execution would return to the next statement after a GOSUB.
    Actually QBASIC (Microsoft BASIC) also allows RETURN to a different line number or line label, and I have used that feature, for a few different reasons. One was to simulate the effect of the INTERCAL command PLEASE RESUME #2
  • Greg 2010-08-25 16:06
    TRWTF is that loops and functions are just syntactic sugar. That's why I like to use GOTO functions and push and pop all of my variables onto the stack manually.
  • wtf 2010-08-25 16:09
    Greg:
    TRWTF is that loops and functions are just syntactic sugar. That's why I like to use GOTO functions and push and pop all of my variables onto the stack manually.


    The real WTF is that you're using variables instead of just remembering where your data is and using the address.
  • Jimmy Hoffa 2010-08-25 16:09
    Not a WTF. Union rules say that you must have at least 4 breaks per function. A for loop was the only way to fulfill this requirement.
  • Bert Glanstron 2010-08-25 16:25
    Jimmy Hoffa:
    Not a WTF. Union rules say that you must have at least 4 breaks per function. A for loop was the only way to fulfill this requirement.

    Dear Jimmy Hoffa,

    In case you can’t tell, this is a dangerous place. The
    fact that you insist on trying to cheat the mafia
    clearly shows that you’re too dead and too missing
    to be organizing unions.

    Go away and rest in peace.

    Sincerely,
    Bert Glanstron
  • swordfishBob 2010-08-25 16:28
    You will not use GOTO! Or else...!
  • PseudoNoise 2010-08-25 16:35
    Hey, remember when the comments in TDWTF were less than 90% memes and talked about the code at hand?

    Good times .... good times...

    I vote for refactor into method suggested on the 1st page. I've seen code style guides that say "only have one return point from a function" but that's just proof those style guides need to die in a fire.
  • Java Expert 2010-08-25 16:41
    Fixed for Java
    boolean bCreateModel = false;
    
    try
    {
    if (pModel != 0)
    {
    throw new RuntimeException();
    }

    if (asModelParts.size() != asModelPartsToLoad.size())
    {
    throw new RuntimeException();
    }

    for (int i = 0; i < asModelPartsToLoad.size(); ++i)
    {
    if (!asModelPartsToLoad.get(i).equals(asModelParts.get(i));
    {
    throw new RuntimeException();
    }
    }
    } catch (RuntimeException e) {
    bCreateModel = true;
    }
  • Schol-R-LEA 2010-08-25 17:03
    Rick:
    pitchingchris:
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.



    From what I'm reading, it was intended to be an april fools joke and never got removed.

    The Pick BASIC language has a 'RETURN TO' statement, which was not implemented as an April Fools joke. The reader of a Pick BASIC program could not assume that execution would return to the next statement after a GOSUB.

    That almost sounds like someone had heard about Continuation Passing Style and decided to try it in the worst way imaginable. Either that, or they were assembly programmers used to being able to munge the stack as they pleased, and missed being able to do it in HLLs. Either way, it's ugly, ugly, ugly... but then, Pick was pretty ugly in general from what little I recall of it.
  • Schol-R-LEA 2010-08-25 17:10
    mh:
    Moshe:
    This is actually a pretty common coding convention. It is not an elaborate GOTO but a poor man exception handling.

    And there I was thinking that exception handling was nothing but an elaborate goto to begin with...


    (Exceptions are
    (a crutch)
    (for (those
    who
    (can (not handle)))
    (first-class continuations)))
  • Coyne 2010-08-25 17:37
    I don't know, but it seems like the point of the WTF is being missed here.

    One of my persistent habits is that of trying to find the general solution. (For example, trying to come up with something that kills everything from a flea to an elephant when what I really need is to just step on the cockroach of the moment.)

    I could see any of the above I guess, if there were thousands or even dozens of things to be checked in a logic sequence. (But then you probably need to refactor something a bit, right?)

    In this case, there are only 3 things to check, and no structured flow violations. So it seems to me that one possible good solution for this problem is:


    bool bCreateModel;
    if (!pModel)
    {
    bCreateModel = true;
    }
    else if (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }
    else
    {
    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    break;
    }
    }
    }



    Now isn't that nice and readable? Even if it isn't showy?

    Taking that into account, the original code author was "federalizing the simple": Used a wrecking ball to kill the cockroach. WTF!
  • Captain Normal Form 2010-08-25 17:39
    Schol-R-LEA:
    mh:
    Moshe:
    This is actually a pretty common coding convention. It is not an elaborate GOTO but a poor man exception handling.

    And there I was thinking that exception handling was nothing but an elaborate goto to begin with...


    (Exceptions are
    (a crutch)
    (for (those
    who
    (can (not handle)))
    (first-class continuations)))


    Continuations are too general for that task. Why embed a computation in a continuation monad (which brings in zero domain related methods, specifically because of how general it is) when you can embed it in an error monad (which brings in exception methods like try, catch, etc)?
  • da Doctah 2010-08-25 17:42
    v.dog:
    I found the problem- this line was meant to be a comment:

    for (;;)

    The code is designed to make you cry.


    It should have been:

    for (;;) {}

    That's the "do-nothing-forever" construct, a metaphor for so many things in this life.
  • Schol-R-LEA 2010-08-25 18:12
    Captain Normal Form:
    Schol-R-LEA:
    mh:
    Moshe:
    This is actually a pretty common coding convention. It is not an elaborate GOTO but a poor man exception handling.

    And there I was thinking that exception handling was nothing but an elaborate goto to begin with...


    (Exceptions are
    (a crutch)
    (for (those
    who
    (can (not handle)))
    (first-class continuations)))


    Continuations are too general for that task. Why embed a computation in a continuation monad (which brings in zero domain related methods, specifically because of how general it is) when you can embed it in an error monad (which brings in exception methods like try, catch, etc)?

    While I was being a bit silly, this actually is a very good point. Part of the reason continuations are difficult to use is because they are far too general and powerful a construct for everyday use. Like GOTO, they are both too primitive and too flexible to be used as is without lot of programmer discipline, and offer endless potential for mistakes and abuse. While I like continuations for the power they offer, in most cases they are better used as a primitive for some more restricted construct.

    OTOH, the fact remains that most programmers aren't familiar with continuations, or even the languages which support them. This is a shame, because they really are a fascinating construct, and (like many of the tools in functional programming) knowing about them changes you perspective on what things like expressions and functions really are.
  • Seahen 2010-08-25 19:07
    I once had to implement an isEmpty() method in JavaScript, and the only way I could find to do it in constant time was

    for (item in this) {
    return false;
    }
    return true;
  • Matthew Chaboud 2010-08-25 19:43
    One of the reasons to use a construct like this (though I've generally thought that do{}while(0) is cleaner) is that goto statements can't skip forward over initialization.

    So, if you want to localize your initializers so your code is both readable and easy to refactor, and you want to dodge having deep exit-out if blocks, a loop-based construct like this can be useful. Also remember that, in ANSI C++, goto was restricted to labels within the current block. No destructor-ness was accounted for.

    do{}while(0), while it still only lets you know that it's a faux-loop at the end, at least doesn't run the risk of turning into an infinite loop if you forget to put a break in the default clause. I typically prefer it to infinite for loops.

    Just put a comment at the top of the block indicating what you're up to and it's a pretty friggin' reasonable thing for anyone but a complete novice programmer.
  • Simon 2010-08-25 20:15
    As others have said, do{...}while(0) would be better than for(;;), but this is really not a WTF. This sort of structure is fairly common. The main advantage is that your function has a single point of exit, which can make it much easier to debug. Since you know that every part of your function will cross that point regardless of what error may have occurred, you can use that to free up any memory which may have been allocated at that single point, which makes tracking down and fixing memory leaks much easier.

    Think of it as C's version of try & catch.
  • Troy 2010-08-25 20:41
    averageGuy:
    The Nerve:
    bool bCreateModel = false;
    
    if (!pModel || asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize() && !bCreateModel; ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    }
    }

    Fixed?


    Almost, but not quite.

    Your code would execute the for loop when the first condition was true. This would at least be wasteful and might lead to errors if !pModel meant that asModelPartsToLoad was not initialized.

    Putting an the for loop into an else clause would fix that.


    I guess you didn't notice that !bCreateModel in the condition of the loop. It works, unless GetSize() has some unwanted side-effect. However, I don't understand why he just didn't make the loop the else clause of the if.
  • Supercoder 2010-08-25 20:46
    Code is perfectly OK.

    It avoids unnecessary testing after bCreateModel has been set, and it skips the extra test which would be required to test if bCreateModel was set before each step.

    Since this is C++, it's a good rule of thumb to avoid gotos to transfer control between scopes, since any destructors that should have been run at the end of the scope won't be run.

    A do { } while(0) loop is preferable, but only because you don't need to remember the final break statement.

    Only WTF is blaming someone for writing bad code, because one is unfamiliar with this construct.
  • Lazy Evaluation is your friend 2010-08-25 20:48


    bool bCreateModel = false;
    UINT32 i = 0;
    UINT32 j = 0;

    bCreateModel = !pModel || (j = asModelPartsToLoad.GetSize()) != asModelParts.GetSize();

    while(!bCreateModel && i < j)
    {
    bCreateModel = asModelPartsToLoad[i] != asModelParts[i];
    i++;
    }

  • Wyrdo 2010-08-25 21:13
    JJ:
    For everyone who has had the "GoTo is Evil" Kool-Aid forced down their throats, I'll repeat my mantra: "No language feature is evil. It is the misuse of a language feature that is evil."


    (In reality, I agree with you, but I have to go with the general meme of one-ups-manship and oh-yeah-ya-think-so here on the site.)

    Clearly you haven't seen INTERCAL. It's very hard to not misuse any of its "features".
    --
    Furry cows moo and decompress.
  • Dennis Ritchie 2010-08-25 22:08
    ais523:
    I've actually done this with a do{}while(0) loop in C, because it was clearer than the alternatives. (This was generated code, btw; the generated version was meant to be readable, but the original was the bit you actually edited.) It's a nice paradigm for what would otherwise lead to 20 nested if statements.


    If you have C at your disposal, you can just use goto.
  • Coder 2010-08-25 22:10
    TheCPUWizard:
    GOTO may not be evil, but the "COMEFROM" statement (bonus points for the first person to identify the late 1970's language that had it) was pure evil.

    It basically acted as a interceptor so that when the specified item was about to execute control would be transfered to the specified target, which could then decide if it was to return to the original statement or the one after it. Making things even more insane was the ability to have these in conditionals!



    I don't think you ever debugged an enterprise app written in DEC BASIC.

    1000 Every language that has a label or line-number
    1010 that you can GOTO has a COMEFROM problem. It is
    1020 not the GOTO that is the problem (although a
    1030 computed GOTO is a bit of a problem). GOTO 1050
    1040 OK, WHERE DID THIS LINE COME FROM ?
    1050 It is the come-from that is the problem.

    Languages that have loop and control structures (structured programming languages) clarify how you get to each line. Structures where the conditional is implemented at the top are implicitly (GOTO 1040) COMEFROM structures. Despite what Wikepedia claims, this does not make them more difficult.

    Note: this comment has a bug.
  • Lisp Weenie 2010-08-25 22:43
    dave:
    Java has the nice, largely unused feature of labeled blocks, which can be used in these cases (when you're too lazy extract the logic to another method).

    determineCreate: {
    if (!pModel)
    {
    bCreateModel = true;
    break determineCreate;
    }
    ...
    }


    Like all good programming language features, Lisp had it first and did it better:


    (block determine-create
    (unless p-model
    (setq create-model t)
    (return-from determine-create))
    ...)


    Not that it would actually be necessary to use this structure in Lisp, given that Lisp allows the following, much more elegant solution, which takes advantage of the built-in structural equality operator:


    (let ((create-model (or (not model)
    (not (equalp model-parts model-parts-to-load)))))
    ;; create-model is defined correctly in only 2 LOC, bitch.
    )


    Captcha: odio, Spanish for "I hate". Odio Java.
  • Xamuel 2010-08-26 00:11
    People have been asking why do{...}while(0) is useful in C. One reason is to force macros to have the same syntax as functions. Consider the line:

    for(i=0;i<5;i++)
    MYMACRO(i);

    If MYMACRO is a naive multi-line macro, then we've got trouble: only the first line will be associated with the for! (Also, the semicolon is superfluous, though that's not fatal.)

    So instead, you define MYMACRO to be do{...}while(0), which transforms multiple lines into one line, and then the above construction works fine. (Also, the semicolon is no longer superfluous)
  • Tuna 2010-08-26 05:08
    ais523:
    I've actually done this with a do{}while(0) loop in C, because it was clearer than the alternatives. (This was generated code, btw; the generated version was meant to be readable, but the original was the bit you actually edited.) It's a nice paradigm for what would otherwise lead to 20 nested if statements.


    If you want to use a goto, use a goto. Replacing it with misused structure is not at all better.
  • Cbuttius 2010-08-26 06:16
    averageGuy:
    The Nerve:
    bool bCreateModel = false;
    
    if (!pModel || asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }

    for (UINT32 i = 0; i < asModelPartsToLoad.GetSize() && !bCreateModel; ++i)
    {
    if (asModelPartsToLoad != asModelParts[i])
    {
    bCreateModel = true;
    }
    }

    Fixed?


    Almost, but not quite.

    Your code would execute the for loop when the first condition was true. This would at least be wasteful and might lead to errors if !pModel meant that asModelPartsToLoad was not initialized.

    Putting an the for loop into an else clause would fix that.


    or just switching the order of the conditionals in the second part of the for loop.

    I am pretty certain this is C++ because of the UINT32, the GetSize() and the use of the comparison operator !=.

    It is incorrect usage of C++ but complies with the language.


    [i]Addendum (2010-08-26 06:26)
    :
    The correct way to do it is the way I outlined on Page 1 it must have been too clsoe to the bottom of the page so I will try to post the correct implementation again. You can do all of this in one statement.

  • Cbuttius 2010-08-26 06:33
    Step 1: Get rid of all the nonsense Hungarian notation and use standard STL collections.

    Step 2: Refactor that into one statement:


    createModel = !model
    || modelPartsToLoad.size() != modelParts.size()
    || !std::equal( modelPartsToLoad.begin(), modelPartsToLoad.end(), modelParts.begin() );



    If you can't refactor into STL collections there are still ways to create proper iterators for it so you can use std::equal. &modelPartsToLoad[0] might work if the collection is guaranteed to be contiguous.

  • pitchingchris 2010-08-26 08:43
    Xamuel:
    People have been asking why do{...}while(0) is useful in C. One reason is to force macros to have the same syntax as functions.


    Why make it so complicated. If you want to force it to run at least one iteration, use do/while, if you want to check the condition before doing any iterations, use while or a for loop.
  • kastein 2010-08-26 16:55
    Coder:
    1000 Every language that has a label or line-number
    1010 that you can GOTO has a COMEFROM problem. It is
    1020 not the GOTO that is the problem (although a
    1030 computed GOTO is a bit of a problem). GOTO 1050
    1040 OK, WHERE DID THIS LINE COME FROM ?
    1050 It is the come-from that is the problem.


    Note: this comment has a bug.

    Error 18 on line 1000: Function not defined: Every
    Error 18 on line 1010: Function not defined: that
    Error 18 on line 1020: Function not defined: not
    Error 18 on line 1030: Function not defined: computed
    Error 18 on line 1040: Function not defined: OK
    Error 18 on line 1050: Function not defined: It

    Those are the only bugs I could find :(

    had forgotten my BASIC error numbers, googled for a reference and good lord! Someone is working on a 64 bit edition of qbasic that supports Linux and Windows. I felt dirty after discovering this.

    Corey:
    my idea:

    Regex exp = new Regex(
    @"(i(\'ve)*(\s)+.*.*you(\'re|r).*(pray).*further)|(bert.*g(.){1,5}ron)|(f[ri1][ri1]st)",
    RegexOptions.IgnoreCase)

    if(exp.IsMatch(CommentText)){
    //TODO: Insert code that will find the poster and urinate on them
    }

    How well do you know javascript / DOM trees, are you familiar with greasemonkey, and how much beer do I owe you?
  • Design Pattern 2010-08-26 17:12
    newfweiler:

    I remember the proposal to add COMEFROM, COMEFROM DEPENDING ON, (computed COMEFROM) and conditional COMEFROM to COBOL and ForTran.

    The only clean implementation of COMEFROM was in Threaded INTERCAL.

    newfweiler:

    The modern language that has COMEFROM is Java, only it's called "Annotations".

    Nope.

    What it's really called is "Aspect-Oriented Programming".

    newfweiler:

    It's useful for application platforms where you want to call your own method from platform code, without modifying the platform code to add a line to call your method.

    But in that case the developer of the platform code KNOWS that he will jump into other people's code.
  • zarg 2010-08-26 17:21
    This is a variation of a pattern that allows error handling in a single place if you're using a language like C that doesn't support exceptions. It's probably better to use a goto:


    model_t* pModel = 0;
    part_t* pParts = 0;

    pModel = GetModelinstance();

    if (!pModel)
    {
    goto error;
    }

    pParts = GetParts( pModel);
    if (!pParts)
    {
    goto error;
    }

    // etc...


    PartsRelease( pParts);
    ModelRelease( pModel);
    return 0;

    error:
    if (pParts)
    {
    PartsRelease( pParts);
    }
    if (pModel)
    {
    ModelRelease( pModel);
    }

    return -1;


    However, there are *a lot* places where using a goto even where it makes sense is irrationally considered very bad practice. In a situation like that, it might make sense to use the break statement as in the example.

    A better way to handle that situation (if the "goto error;" isn't permitted or desired) might be to use a "do {} while (false)" statement instead of a "for (;;) {}".
  • ateu 2010-08-26 18:07
    DE-WTF:

    if (!pModel)
    {
    bCreateModel = true;
    }
    elseif (asModelParts.GetSize() != asModelPartsToLoad.GetSize())
    {
    bCreateModel = true;
    }
    else for (UINT32 i = 0; i < asModelPartsToLoad.GetSize(); ++i)
    {
    if (asModelPartsToLoad[i] != asModelParts[i])
    {
    bCreateModel = true;
    break;
    }
    }
  • MMXII 2010-08-26 21:33
    You don't have to complete the inner loop as it's just looking for the first occurrence of inequality.
  • SurturZ 2010-08-27 00:36
    It amazes me how stupidly people will code, just to avoid a GOTO.

    http://slashdot.org/journal/145833/In-defense-of-GOTO

    The "Do ONCE" construct is a common anti-pattern.
  • Cbuttius 2010-08-27 06:58
    SurturZ:
    It amazes me how stupidly people will code, just to avoid a GOTO.

    http://slashdot.org/journal/145833/In-defense-of-GOTO

    The "Do ONCE" construct is a common anti-pattern.


    The scenario referred to in the article is only applicable for the C language.

    In C++ you have RAII, which effectively involves creating scoped objects whose destructors invoke when they go out of scope. As a result you do all your "clean-up" in those.

    There there is no need for goto or finally in try-catch blocks.
  • Tanuki 2010-08-27 15:30
    Let us see, gotos are uncivilized and the workarounds just plain ugly. I know! I use goto but wrap it up to some macros to make it more structured. At least it directly spells no goto in the source file, so aren't you happy?

    /*T stands for trap*/

    typedef void* TLEAP;

    #define TLEAP_INIT(leaptype) (leaptype = NULL)

    #define TLEAP_DESTINATION(label_name) _label_##label_name:

    #define TLEAP_RESET(leaptype) (leaptype = NULL)

    #define TLEAP(ticket,label_name) \
    do{\
    ticket=&&_CC(_tleap_,__LINE__);\
    goto _label_##label_name;\
    _CC(_tleap_,__LINE__):;\
    ticket = NULL;\
    }while(0)

    #define TLEAP_RETURN(ticket) do{if(ticket) goto *ticket;}while(0)

    #define _CC(A,B) _CC2(A,B)
    #define _CC2(A,B) A##B


    Used in a simple parser program.
  • MR O 2010-08-31 09:45
    Actually, I have also seen this kind of code, except that the programmer mask it in macros like

    BEGIN_BS
    BREAK_IF( cond );
    BREAK_IF_WITH( cond, var = value );
    END_BS

    I first read the BS as you know, BS. And I think it is utter BS.
  • Shinobu 2010-08-31 11:00
    Sure, this particular case might have been structured better, but in general this isn't a bad pattern. In VB, I've seen Do ... Loop While False used a lot, especially where some things need to be done, then a check, then some more actions, another test, and so on. Such code is much clearer than the equivalent with Gotos, and a lot less Alpine than nested Ifs. The language just doesn't have Do ... Once and you can't blame the coder for that.
    In C++ you might do something like ‘#define once while(false)’ although then ‘once { ... }’ won't do what you expect at all... You might ‘guard’ yourself against this with an extra semicolon, but that actually causes a more insidious problem. All in all, I prefer not to use #defines this way.
    zzo38:
    Actually QBASIC (Microsoft BASIC) also allows RETURN to a different line number or line label, and I have used that feature, for a few different reasons. One was to simulate the effect of the INTERCAL command PLEASE RESUME #2
    You know you're hard core when you're missing your favourite Intercal command.
  • Lo'oris 2010-08-31 13:11
    Uh?

    This is quite common, actually.

    And quite useful, too.

    GOTO will not eat your soul, despite what the Church of the Holy Syntax might say, and tricks such as this one are even more harmless.

    Did you really say you would have preferred three nested ifs? WTF?
  • INTERNETS 2010-09-01 02:48
    It's possible that this is just for historical reasons. It may have originally done something in the for() loop which was removed, leaving the code looking like that.
  • Shinobu 2010-09-01 06:12
    (replying to myself)
    ‘#define once while(false);0’ works, since the 0 can't be followed by an { or most statements, so the temptation of writing ‘once { ... }’ won't be there.
    But is harmless when followed by a ; except when the next statement is an else: ‘if(...) do { ... } once; else ...;’ is invalid.
    It could be rewritten as ‘do { if(!...) { ...; break; } ... } once;’ but I still think that the preprocessor solution isn't ideal and I won't use it.
  • distineo 2010-09-02 07:40
    Jay:

    Seriously, though, this is my problem with people who insist on following a dogmatic rule without bothering to udnerstand why the rule was invented in the first place.


    Yet you fail to elaborate the reasons why the rule was invented. Goto isn't evil because it is spelled g-o-t-o.

    One reason not to use goto is that program flow can jump to anywhere. You have to find the label to know where execution is going to jump to.

    Not with 'do {} while (0);' - this jumps to the end of the block which is easily located.

    When you encounter a label in the source, the gotos leading there may again be anywhere in the code. No problem with 'do {} while (0);' because all the breaks have to be in the braces.

    Jumping into a construct is an issue with a real goto but it isn't possible with 'do {} while (0);'.

    Calling 'do {} while (0);' a goto is misleading, nay, outright wrong, except if you call it a structured goto ;-)

    Personally I prefer 'switch (0) { default:' on one line, and a comment above it. The 'while ()' and 'for (;;)' constructs make people expect a loop that isn't there - and there is even the theoretical risk of an infinite loop if you manage to misplace a 'continue'.

    There are lots of things that "everybody knows" that have no foundation.
  • letatio 2010-09-02 07:43
    Stuck in the Tarpit:
    Everyone accepts that it's wrong to have a function with multiple return points. Many code-scouring tools identify that as a problem.


    I don't, and I make my own opinion instead of taking a tool's output for canon.

    A return ensures that execution flow cannot reach those points that it should not reach, so there's no need for elaborate if () conditions or else chains.
  • dgghua 2010-09-04 04:14
    Good article you post! It enlarge my knowlege on the point!Thank You for the post. I love to read interesting post that has knowledge to impart. I hope to read more articles from you and in return I will post also my articles in the forum so that others can benefit from it. Keep up the good work!
  • dgghua 2010-09-04 04:17
    Welcome to RosettaStone-counter.com - the best Rosetta Stone supplier on the Internet.Rosetta stone learning software worthy of trust, the effect is very obvious, guarantee our product, progress quickly!Rosetta Stone on sale,you can get the discount Rosetta Stone form RosettaStone-counter.com,free and fast shipping,PayPal payment!
    <a href="http://www.rosettastone-counter.com">Cheap Rosetta Stone software </a>
    <a href="http://www.rosettastone-counter.com/rosetta-stone-english-c-9.html>Rosetta Stone English </a>
  • Tanuki 2010-09-04 10:17
    Or, in a slightly pascalish manner, one could define a single variable where to put the return value and use goto to get to the return statement at the end of the function.

    Though that would equally sic the purists after my ass, which is ironic, as I've heard more than once "a nested function with multiple return points as a structured substitute of goto." Supposedly those people have misunderstood the purest meaning of structured programming.
  • got an idea 2010-09-05 14:50
    well

    i am not sure, why they always use a for look for this construct

    a better way could have been another loop:

    do
    {

    if 1... break;
    if 2... break;
    break; <--- can now be ommited
    } while (FALSE);

  • random 2010-09-08 09:35
    NASA Engineer:
    No, you guys don't get it. This loop works like the "slingshot" maneuver that probes do to pick up speed before leaving orbit. Using a loop and then breaking out of it actually makes the code run faster!


    ROTFLOLOLOLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOLLOL
  • gbonehead 2010-09-08 13:24
    The main issue here is that it's a borderline case.

    Consider what's really happening.
    if (check1)
    
    bCreateModel = true;
    else if (check2)
    bCreateModel = true;
    else
    /* Do expensive test to see if bCreateModel should be true.*/

    Sure, that could be done as an if/else with little change.

    But really. Multiple functions? Multiple returns from a function? For this code? Personally, I go through code where people have lazily peppered returns randomly all over their functions and refactor them to have single flow - for me functions are much easier to follow when they enter and exit in one place, not to mention easier to handle in a debugger, something that far too few developers actually use. With a single point of return, there's none of this "Oh ... I didn't see that return buried in the middle of that if/else in the for loop."

    I would not use the construct given for something that simple; in fact I wouldn't use it at all because
    for (;;)
    is very commonly used to mean "forever".

    On the other hand, I use do{}while(0); very often for cases such as the following:
    bool succeeded = false;
    
    do
    {
    doExpensiveInitialization();
    if (fails)
    break;

    doExpensiveInitialization();
    if (fails)
    break;

    ...
    succeeded = true;
    }
    while(0); // For control flow only
    return(succeeded);

    All the if/else-happy happy folks would be stuck doing something like the following (which I've seen far too many places in our code):
    bool succeeded = false;
    
    if (!fails)
    {
    doExpensiveInitialization();
    if (!fails)
    {
    doExpensiveInitialization();
    if (!fails)
    {
    /* ad nauseum */

    succeeded = true;
    }
    }
    }
    return(succeeded);

    Note: the real WTF is that the Java folks realized what the C++ folks didn't - a simple named break solves this quite cleanly without introducing the randomness of a GOTO:
    FAILURE:
    
    {
    doExpensiveInitialization();
    if (fails)
    break FAILURE;

    ...
    }

    Now, that is code that has obvious purpose and design without having to hope that intent is clear because the language is not.
  • me 2010-09-08 22:20
    Ah, c'mon. There are sometimes legit reasons for that sort of construction in C: one-stop resource cleanup. Something like:


    char *a = 0, *b = 0, *c = 0;
    int ok = 0;
    do {
    a = malloc(...);
    ...stuff...
    if (...) break;
    ...more...
    if (...) break;
    b = malloc(...);
    ..blah...
    if (...) break;
    c = malloc(...);
    ok = 1;
    } while (0);
    free(a);
    free(b);
    free(c);
    if (!ok) return;


    It's definitely safer/less cumbersome to have all the cleanup code in one place, rather than writing a 'return' where I have each 'break', and trying to make sure you do the right cleanup in each place.

    Sure, you can do the same with a 'goto' but this is, I think, a little cleaner. It should be self-evident that the final clauses get allocated.

    Something like try-finally would be better, but if your language don't got that, you have to make one up.

    (But note do-while(0) rather than for(ever))
  • M2tM 2010-09-13 18:43
    Let me clarify here a few things about this.

    1) This is C++, not C.
    2) There is a simple way to implement this with a single if/else (replacing the loop/break) which more directly expresses intent.
    3) If you want to go a step further it can be made into its own method.
    4) There is no master plan behind this code, you are not witnessing a common idiom in our code base. Much of the 7000 line file I was roaming through contained reams of magic constants and hard coded logic which should have been represented in configuration files.

    This particular example is ridiculous and if you are defending this style of coding I can only say I'm happily not working with you. A few programmers here I shared this with said wtf and suggested I submit it.
  • Ntx 2010-09-14 11:35
    Yes, that's sublime dude!