• bla (unregistered)

    do{ frist }while(0)

  • (cs)

    That loop was probably a misinterpretation of the requirement "make sure this section of code runs exactly once."

  • Nagesh (unregistered)

    I drunk wiskey many shots night last and smoke bidis and I have handover massive this am.

  • The Nerve (unregistered)

    Fixed?

    int errors=0;          
  try
  {
	// LOG_E_BREAK("");
    	int const nMatrices=m_rawData.nMatricesX()*m_rawDataDim.nMatricesY();
	errors+=m_CCDDataGainArr.SetSize(nMatrices);
	//LOG_E_BREAK("");

  } finally {}
  return errors;
  • The Legacy Hangover (unregistered)

    while(0) is what you get when you get a bash scripter to write a loop

  • (cs)

    It all depends on the definition of LOG_E_BREAK. If it's doing a unilateral test on errors, and printing a (low-value) message and doing a break if it's non-zero, it's just nasty. If it's doing anything else, it's time to page Mr. Lovecraft to take over its maintenance.

  • Joe Tennies (unregistered)

    Actually, my assumption is that it was originally a straight C program with that buried in the middle of a function somewhere. That is quite common in C as you can't declare a variable on the fly without a new scope being opened. That "int const nMatrices..." is why it was done.

  • (cs)

    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct

  • (cs) in reply to A Nonny Mouse
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
  • Michael P (unregistered) in reply to Joe Tennies
    Joe Tennies:
    Actually, my assumption is that it was originally a straight C program with that buried in the middle of a function somewhere.

    Either that, or this snippet was originally a macro. I'm not sure why this particular non-loop would be the thing to break the camel's back; counting the possibility of a "break;" inside LOG_E_BREAK(), there are now three fairly reasonable explanations for why it might be written that way.

  • (cs)

    A wild guess:

    #define LOG_E_BREAK(x) if (errors) { \
    printf("%s\n", x); \
    printf("<current date and time>\n"); \
    break; \
}
  • (cs) in reply to Severity One
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
  • (cs) in reply to Joe Tennies
    Joe Tennies:
    Actually, my assumption is that it was originally a straight C program with that buried in the middle of a function somewhere. That is quite common in C as you can't declare a variable on the fly without a new scope being opened. That "int const nMatrices..." is why it was done.

    Nope, there is nothing that stops you from creating a scope wherever you wish in C:

    int foo(void) {
    int outer = 0;
    outer = 1;
    {
        int greetingsFromInner = 2;
        outer = greetingsFromInner;
    }
    return outer;
}
  • (cs) in reply to Severity One
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?

    Another useless use of cat.

  • (cs) in reply to BentFranklin
    BentFranklin:
    Another useless use of cat.
    cat foo | less
  • Sergey (unregistered)

    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

  • (cs) in reply to Patryk Zawadzki
    Patryk Zawadzki:
    A wild guess: 
    #define LOG_E_BREAK(x) if (errors) { \
    printf("%s\n", x); \
    printf("<current date and time>\n"); \
    break; \
}

    That is much less of a problem if that snipet was originaly the main loop of the code. Problem is, there is no interface handling on it, so it can not be a main loop of something.

    It can yet be explainned by a long time of evolution, while the snippet lost the interface handling, while the "while" was made harmless easlier, so nobody touched it... But WTF! Nobody thought about cutting just the usefull parts of it, instead of the entire thing?

  • (cs) in reply to Patryk Zawadzki

    Exactly. You need less cat food if you use a pipe.

  • (cs)

    Continuous Integration would have fixed the build issues. Good unit tests (including memory leak testing) would have handled the memory leak issues.

    And the logging, while not actually logging any useful message, are indeed useful -- I can look at the log file, and see the where the log statement is just before the program/thread died.

  • Occasional Reader (unregistered) in reply to frits
    frits:
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
    <pedant> Is he any relation to Schroedinger? </pedant>
  • (cs) in reply to frits
    frits:
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
    Nah, it wasn't a piano, just an electronic keyboard. And I thought his name was Fatso.
  • (cs)
    The massive data allocation did cause a few raised eyebrows...
    Exactly how many eyebrows did Paul have?
  • gallier2 (unregistered) in reply to Sergey
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    Yes, my colleague uses that construct, I hate him for that. It's completely retarded. If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto. The break is a goto, it's not because it wears a mask, that it is not a goto. The worse about it, is that a break is for leaving a loop, but here you don't have a loop, so when you see do/break/while, you must considere 2 possibilities, is it or is it not a loop. My colleague even once used that thing, but made really a loop out of it by using a continue in one hidden case (reallocation of memory).

  • trtrwtf (unregistered) in reply to boog
    boog:
    frits:
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
    Nah, it wasn't a piano, just an electronic keyboard. And I thought his name was Fatso.

    And unless he's in the shower, he's a dry Waller.

  • Paul's Girlfriend (unregistered) in reply to boog
    boog:
    The massive data allocation did cause a few raised eyebrows...
    Exactly how many eyebrows did Paul have?

    Honey, Paul's got eyebrows where most guys don't even have places.

  • Marvin the Martian (unregistered) in reply to frits
    frits:
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
    And the cat was called Pauli; he'd always fight so there's at most one in the box.
  • (cs) in reply to Patryk Zawadzki
    Patryk Zawadzki:
    A wild guess: 
    #define LOG_E_BREAK(x) if (errors) { \
    printf("%s (at %s, line %d)\n", x, __FILE__, __LINE__); \
    printf("<current date and time>\n"); \
    break; \
}
    If so, the above changes might be a great way to get started fixing the code.

    Something tells me it wasn't that easy though.

  • (cs) in reply to gallier2
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    Yes, my colleague uses that construct, I hate him for that. It's completely retarded. If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto. The break is a goto, it's not because it wears a mask, that it is not a goto. The worse about it, is that a break is for leaving a loop, but here you don't have a loop, so when you see do/break/while, you must considere 2 possibilities, is it or is it not a loop. My colleague even once used that thing, but made really a loop out of it by using a continue in one hidden case (reallocation of memory).

    Clearly, the code author was a victim of Dijkstraism.

  • The Poop... of DOOM (unregistered) in reply to gallier2
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    The break is a goto, it's not because it wears a mask, that it is not a goto.

    Everything is a goto. A loop isn't much more than: 10. // Shennanigans 11. yourvar++ 12. if (yourvar < iterations_amount) goto 10.

    A function call isn't much more than a masked goto label.

    Everything is a goto.

  • Amateur Troll (unregistered)

    It's a slow day at work...does anyone mind if I start a flame war?

  • (cs)

    The custom string class also seemed to use a custom memory handler as malloc was probably found wanting in some respect. The occasional new was never explained.

    I'm assuming that got garbled, and the custom string class used a custom memory "handler" (or allocator more accurately) instead of plain old new. And I'd certainly expect at least a few occurrences of new in any large C++ app ...

    As for the preprocessor logging and homespun persistence layer, I'm lead to believe that there are decent frameworks for this stuff in C++ land now - there was definitely something written in C that abstracted a lot of the DB stuff away last time I did that kind of thing.

  • ted (unregistered) in reply to Amateur Troll
    Amateur Troll:
    It's a slow day at work...does anyone mind if I start a flame war?

    Fuck you. You're not clever.

  • Sergey (unregistered) in reply to gallier2

    Then you have some learning to do. The problem with goto is not that it's somehow evil, it's that you have to generate a separate label every time, and make sure that you use the correct label in the correct place. You don't want to copy-paste a piece of code and accidentally jump to the end of the wrong group. Break takes care of it for you.

  • MasonicParrot (unregistered) in reply to gallier2
    gallier2:
    Yes, my colleague uses that construct, I hate him for that. It's completely retarded. If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto. The break is a goto, it's not because it wears a mask, that it is not a goto. The worse about it, is that a break is for leaving a loop, but here you don't have a loop, so when you see do/break/while, you must considere 2 possibilities, is it or is it not a loop. My colleague even once used that thing, but made really a loop out of it by using a continue in one hidden case (reallocation of memory).

    Sounds like you should get your rage on with this guy:

    http://www.diag.com/news/DoWhileFalse.html

    Personally, I think it's a fine and useful idiom, no worse than an early return (cue the SESE brigade).

  • (cs) in reply to boog
    boog:
    The massive data allocation did cause a few raised eyebrows...
    Exactly how many eyebrows did Paul have?
    I only have one.
  • HP PhaserJet (unregistered) in reply to The Poop... of DOOM
    The Poop... of DOOM:
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    The break is a goto, it's not because it wears a mask, that it is not a goto.

    Everything is a goto. A loop isn't much more than: 10. // Shennanigans 11. yourvar++ 12. if (yourvar < iterations_amount) goto 10.

    A function call isn't much more than a masked goto label.

    Everything is a goto.

    There's no need for hyper-correction...

    In this particular situation, a goto will actually be clearer and easier to understand than an if statement or loop, and would be more suitable.

    I think that's what he was trying to say, and I agree. Use a stupid goto if you'd have to write something even more stupid to get the same functionality.

  • (cs) in reply to gallier2
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    Yes, my colleague uses that construct, I hate him for that. It's completely retarded. If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto. The break is a goto, it's not because it wears a mask, that it is not a goto. The worse about it, is that a break is for leaving a loop, but here you don't have a loop, so when you see do/break/while, you must considere 2 possibilities, is it or is it not a loop. My colleague even once used that thing, but made really a loop out of it by using a continue in one hidden case (reallocation of memory).

    Your colleague is doubly daft - not sure about the architecture he was targeting, but whether he was actually doing something that would be more optimal at the instruction level is compiler specific (compiler version and setting specific in fact). A quick test suggests he isn't going to see any win on x86:

    $ cat while.c
#include <stdio.h>

int
main(int argc, char *argv[])
{
    if (argc == 2) {
        goto error;
    }
    printf("bitch tits");
    return 0;

error:
    fprintf(stderr, "man boobs\n");
    return 1;
}
$ cat dowhile.c
#include <stdio.h>

int
main(int argc, char *argv[])
{
    do {
        if (argc == 2) {
            break;
        }
        printf("bitch tits");
        return 0;
    } while (0);

    fprintf(stderr, "man boobs\n");
    return 1;
}
$ cc -S -c while.c; cc -S -c dowhile.c
$ wc -l while.s; wc -l dowhile.s
      42 while.s
      42 dowhile.s
$ diff while.s dowhile.s
1c1
< 	.file	"while.c"
---
> 	.file	"dowhile.c"
  • trtrwtf (unregistered) in reply to The Poop... of DOOM
    The Poop... of DOOM:
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    The break is a goto, it's not because it wears a mask, that it is not a goto.

    Everything is a goto. A loop isn't much more than: 10. // Shennanigans 11. yourvar++ 12. if (yourvar < iterations_amount) goto 10.

    A function call isn't much more than a masked goto label.

    Everything is a goto.

    A loop, break, or conditional is implemented with an underlying branch instruction in the object code. That doesn't make it a goto. If you write a lisp interpreter in C, that doesn't mean your lisp is "really" C, and if your C compiler generates ASL, that doesn't mean you're "really" writing C. It's good to know what your object code will look like, but we use compilers precisely because goto is unconstrained and has no semantics, while "if" and "for" and "return" are strictly constrained and have clearly defined semantics. Unless you debug the object code, your loops are not gotos.

  • HP PhaserJet (unregistered) in reply to MasonicParrot
    MasonicParrot:
    Sounds like you should get your rage on with this guy:

    http://www.diag.com/news/DoWhileFalse.html

    Personally, I think it's a fine and useful idiom, no worse than an early return (cue the SESE brigade).

    The presence of the loop implies that there is a loop when there isn't. The presence of the goto shows there is a goto when there is.

  • trtrwtf (unregistered) in reply to MasonicParrot
    MasonicParrot:
    Sounds like you should get your rage on with this guy:

    http://www.diag.com/news/DoWhileFalse.html

    Personally, I think it's a fine and useful idiom, no worse than an early return (cue the SESE brigade).

    I don't think there's anything wrong with an early return - if your functions are too long to keep track of the returns, they're too long, period, and you need to break them up. This "construct", however, is basically an early return with an added level of obfuscation. Just return and be done with it, I say. Not to say there couldn't be cases where it's useful, but I can't think of any.

  • midas (unregistered) in reply to gallier2
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto.

    That makes sense.

    Bot how about extracting the code into a separate method/function that returns early? IMO clearer than both.

  • Hortical (unregistered) in reply to midas
    midas:
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto.

    That makes sense.

    Bot how about extracting the code into a separate method/function that returns early? IMO clearer than both.

    Even better!

    Hacking the language should really be a last resort, not an established idiom.

  • (cs) in reply to trtrwtf
    trtrwtf:
    I don't think there's anything wrong with an early return - if your functions are too long to keep track of the returns, they're too long, period, and you need to break them up. This "construct", however, is basically an early return with an added level of obfuscation. Just return and be done with it, I say. Not to say there couldn't be cases where it's useful, but I can't think of any.

    Ugh. </facepalm>

    Early returns are TRWTF (and the devil). When debugging a function with N number of returns, you have to add N number of distinct breakpoints just to debug where a function call exits at; such a waste of time and resources. Come on, which would you rather debug:

    <?php
function myFuncEarly($data) {
 global $some_other_data;

 if (empty($data))
    return false;

 if (!is_array($data))
    return false;

 if ($data != $some_other_data)
    return false;

 return true;
}</pre>

    or
    

    <?php
function myFuncLate($data) {
 global $some_other_data;
 $return = true;
 
 if (empty($data))
    $return = false;

 if (!is_array($data))
    $return = false;

 if ($data != $some_other_data)
    $return = false;

 return $return;
}</pre>

    Both functions do the same thing, but in the former you would have to add 3x as many debugging breakpoints just to know why the function returned false. But, in the latter, you only have to add one debug breakpoint.
    

    Early returns are nothing more than lazy tools for unimaginative programmers.
  • (cs) in reply to Hortical
    Hortical:
    midas:
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto.

    That makes sense.

    Bot how about extracting the code into a separate method/function that returns early? IMO clearer than both.

    Even better!

    Hacking the language should really be a last resort, not an established idiom.

    Hortical:
    midas:
    gallier2:
    Sergey:
    do { } while(0) is a common idiom. The idea is that if you want to exit early on error, you just do a break and avoid a goto. And also if you want to put multiple statements in a macro.

    If you conceptually need a goto (and an error exception is a good reason for a forward goto), do a goto.

    That makes sense.

    Bot how about extracting the code into a separate method/function that returns early? IMO clearer than both.

    Even better!

    Hacking the language should really be a last resort, not an established idiom.

    Indeed. It's like people who use:

    function_call1() || function_call2();

    instead of

    if (!function_call1()) function_call2();

  • (cs)

    To me this looks like someone thought they'd be clever and run the code through gcc -E to speed up compile times or something, and thus expanded a bunch of macros, one of which had some broken "log nothing" behavior in whatever particular combination of compile-time defines were set at the time.

  • Hortical (unregistered) in reply to dohpaz42
    dohpaz42:
    Ugh. </facepalm>

    Early returns are TRWTF (and the devil). When debugging a function with N number of returns, you have to add N number of distinct breakpoints just to debug where a function call exits at; such a waste of time and resources. Come on, which would you rather debug: ....

    So you're using a permanent change to the code (which increases the chances for screw-ups, like the screw-up in your code sample) to solve a temporary problem (use with debugger) when you could just change how you use the bugger?

    Try setting a break point at the top of the function and stepping through it, once you've detected that there is a problem in that function?

  • Dude (unregistered) in reply to Occasional Reader
    Occasional Reader:
    frits:
    Severity One:
    A Nonny Mouse:
    it's heisenberg's principle error logging: you can record the datetime of the error, or the error detail, but never both. making your response to the question "what went wrong?" of "i'm not sure" even more correct
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Nah, you're thinking of that kid that plays piano. I think his name is Schroeder.
    <pedant> Is he any relation to Schroedinger? </pedant>

    Nope, just Schroedinger's Cat.

  • c# guy (unregistered) in reply to dohpaz42

    or you could just put a break point of the closing brace of the function.

    returning early is generally preferable to trying to track some variable throughout the life time of the function.

  • HP PhaserJet (unregistered)

    I wonder if this snippet of mine is going to piss anyone off:

    	public bool Read()
	{
		again:

		if (!Reader.Read()) return false;

		// Ignore whitespace and comments
		if ((Reader.NodeType == XmlNodeType.Whitespace) ||
			(Reader.NodeType == XmlNodeType.Comment))
		{
			goto again;
		}

		Reader.MoveToElement();
		return true;
	}

    It's got a goto and a conditional with a side-effect!

    Never had any problem debugging it, even when there were bugs. Never had any problem reading it.

  • Jerry (unregistered) in reply to Severity One
    Severity One:
    Heisenberg is the one of the cat, right? That if a cat is in a closed box, you can't measure its velocity?
    Well almost. The smaller you make the box, the faster the cat is probably going. But you can never be entirely sure.

Leave a comment on “The Legacy Handover”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article