#define DEBUG



  • I am working in C++ writing a small windows app. I would like to add lines only in a debugging build, mainly a lot of error checking.

    Error is a class with a constructor taking four arguments, an error priority flag, a message string, the line number, and the file the error occurred in.

    I have written a macro the adds the __LINE__, and __FILE__ macros into a throw Error(...) statement. 

    Currently my code looks something like this: 

    bool DrawSomething(HWND hWnd)

        HDC hdc = GetDC(hWnd);

        #ifdef DEBUG_BUILD

        if(hdc == null) throw Error(ERROR_PRIORITY_FATAL, "Could not create a valid hdc");

        #endif 

        //draw to the window here

       return true;

    }
     

    If I want the debugging build, I just define DEBUG_BUILD in a global header file, it works fine, but is clunky and I have heard people say to avoid mixing preprocessor macros and code like this. Makes sense, I can see how this might add some hard to find bugs.

    My question is, is there a better way of doing this? should I just forget about it entirely? After all, no user is going to notice a few extra processor cycles. On the other hand, if someday I needed to do this in a place where it was speed-critical, it would be nice to know a better way.
     



  • Well, wouldn't you want to thow the exception regardless?  What happens when you do a release build and hdc is NULL?

    Ignoring that for the moment, what you could do is something like this:

    #ifdef DEBUG_BUILD
    #define THROW_ERROR(priority, message) throw Error(priority, message);
    #else
    #define THROW_ERROR(priority, message) ((void)0)
    #endif
    

    or

    void THROW_ERROR(int priority, const char* message)
    {
    #ifdef DEBUG_BUILD
    throw Error(priority, message);
    #endif
    }

    bool DrawSomething(HWND hWnd)
    {
    HDC hdc = GetDC(hWnd);
    if(hdc == NULL)
    THROW_ERROR(ERROR_PRIORITY_FATAL, "Could not create a valid hdc");
    ...
    }



  • My opinion is that this form is fine _if_ it's code you really only want to happen in debug mode -- like outputting additional information...but in this case what happens in your app if context ends up null in release mode?  It'll just crash; in a very real way you're saying "Only handle errors when I'm in debug mode, otherwise crash" :)   Spend the very few additional cycles to get that error handling consistently...

     -cw 



  • Looks to me like you would want to throw the exception whatever mode you are in.

    In general though you should not set this processor in a big header file, you should set it in the build.

    By the way, if it's not bad enough that pre-processors are used, it annoys me exceedingly when these
    are defined in the program somewhere based on other pre-processors. If I look at some code that doesn't
    compile I can usually see what the compiler is checking for at that point. I can't always then go and look
    up where that is defined in the code. I assume I must set it myself.

    (btw, it's a big WTF to me that I have to set pre-processors just to get it to compile with a standard compiler.
    It's those using non-standard compilers that should have to set them).



  • Thanks guys, I think I will just forget about removing the line from the production build. It isn't worth it just to save a few clock cycles, guess I got carried away with that one.

    I have another question though. I write my if statements with the bracket on it's own line like this:

    if(condition)
    {
       //Do stuff here

    }

    I have heard people say it is better to do something like this:

    if(condition){
     //Do stuff here
     }

    to me, that is less readable. Is this a standard practice that is worth adopting just because most people will expect the second version? 

     

     



  • I very much prefer the former, as I feel it's much more readable; I get quite irate when I see the latter. The latter format *does* conserve a line of space per code block, which used to be important back in the 24-line text editor days. These days you have high-resolution monitors and small point fonts on your IDE, so it doesn't matter as much.



  • [quote user="makito"]

    I have another question though. I write my if statements with the bracket on it's own line like this:

    [/quote]

    We've already had this discussion, and I don't think there is a clear concesus about that issue. Some people prefer it one way, some the other. IMO the best advice: Use the style that has been used in the rest of the project(s) you are working on.



  • [quote user="makito"]

    Thanks guys, I think I will just forget about removing the line from the production build. It isn't worth it just to save a few clock cycles, guess I got carried away with that one.

    I have another question though. I write my if statements with the bracket on it's own line like this:

    if(condition)
    {
       //Do stuff here

    }

    I have heard people say it is better to do something like this:

    if(condition){
     //Do stuff here
     }

    to me, that is less readable. Is this a standard practice that is worth adopting just because most people will expect the second version? 

     

     

    [/quote]

     

    That question just begs to start a flame war.  It really is a matter of style and is therefore highly subjective.  IMO, the only thing that matters is that the style is consistent across the project.

     



  • [quote user="makito"]

    I am working in C++ writing a small windows app. I would like to add lines only in a debugging build, mainly a lot of error checking.

    Error is a class with a constructor taking four arguments, an error priority flag, a message string, the line number, and the file the error occurred in.

    I have written a macro the adds the LINE, and FILE macros into a throw Error(...) statement. 

    Currently my code looks something like this: 

    bool DrawSomething(HWND hWnd)

        HDC hdc = GetDC(hWnd);

        #ifdef DEBUG_BUILD

        if(hdc == null) throw Error(ERROR_PRIORITY_FATAL, "Could not create a valid hdc");

        #endif 

        //draw to the window here

       return true;

    }
     

    If I want the debugging build, I just define DEBUG_BUILD in a global header file, it works fine, but is clunky and I have heard people say to avoid mixing preprocessor macros and code like this. Makes sense, I can see how this might add some hard to find bugs.

    My question is, is there a better way of doing this? should I just forget about it entirely? After all, no user is going to notice a few extra processor cycles. On the other hand, if someday I needed to do this in a place where it was speed-critical, it would be nice to know a better way.
     

    [/quote]

    As others have said, nothing in the GetDC documentation suggests it can only fail in some arbitrary "debug" mode, so why would you only want to handle failures safely in debug mode?  Write the error handling code.  Until you've profiled it, it's not "too slow" to be in the production version.
     

    In general you should make your debug version as close to identical to the release version as possible -- after all, your end users will be beating on the release version, not the debug version, and end users are guaranteed to find the bugs you thought weren't there.  Why risk introducing more by making the code path different for them than for testing?

    For things that should never-ever-ever fail, you can use the standard assert() macro in <assert.h> (in C) / <cassert> (in C++) which is eliminated at compile time if and only if the macro NDEBUG is passed.  Keep in mind that a failed assert calls abort(), normally, ending the program immediately: use it for validating assumptions, not for checking for failure.  For instance,

    int my_func (struct mytype *some_ptr) {

      assert (some_ptr);

      ...

    }

    is okay, if the contract for my_func says callers must not pass NULL.  But this,

    int my_other_func (unsigned n) {

      int *workingArray = malloc (n * sizeof (int));

      assert (workingArray);

      ...

    }

    is not okay, because malloc can return NULL during normal operation.

    Oh, and asking for opinions on brace style is a really, really bad idea.


    Angstroem



  • I didn't know that this was such a controversial issue. This is my first post to the forum, just write it off as a newbie asking a stupid question. :)



  • definitially do not make error checking only compile in the debug version.  you don't really want core dumps in the release build after all! 

     

    you were asking for a way to only do certain things in debug mode.  well, probable you will want your end users to be able to send you a debug log if they find a bug.

     

    do:

    int debug = getenv("MYAP_DEBUG");
    <code>
    if (debug)
    {
        fprintf(DEBUG_FILE, "blah blah blah");
    }

    <code>
     



  • [quote user="makito"]I didn't know that this was such a controversial issue. This is my first post to the forum, just write it off as a newbie asking a stupid question. :)
    [/quote]

    It can cause friction in the workplace too. I used to have a lot of issues with K&R coders. I would refuse a job if I were forced to code in that style.

    You will probably find if you search the internet that the issue has been discussed many times over the years and usually leads to someone saying "oh no, not that flame war again please".

     


Log in to reply