Is Raymond Chen's use of Assert valid?

| 16 Comments | 1 TrackBack

I'm sure you're all getting bored on my views on using Assert in modern C++ code by now, I'll try and make this the last one. Last time, I asked if anyone had what they'd consider to be an example of the use of assert in C++ where the usage was valid given my objections to assert. Mark updated this recent posting on the subject to include a link to Raymond Chen's blog where Raymond is discussing COM object destruction and uses an assert... Given that everyone knows that Raymond is a seriously good programmer doesn't his use of assert validate the position of those who disagree with me.

No. But then I would say that, wouldn't I...

The code in question is this:

ULONG MyObject::AddRef()
{
   assert(m_cRef != 0);
   return InterlockedIncrement(&m_cRef);
}

The assert is added with the comment "Therefore, at a minimum, you should assert in your AddRef() method that you aren't incrementing the reference count from zero.".

COM is a complicated environment. You generally can't throw exceptions across COM boundaries because the COM runtime doesn't deal with C++ exceptions. Lots of people use this as an excuse not to use C++ exceptions inside COM objects at all, but in reality you just need to make sure that you don't pass an exception out of a COM interface. Most COM interfaces return an error code in the form of an HRESULT which tells the caller if everything was OK (by returning S_OK) or if there were problems (by returning anything else). There are lots of documented COM error codes and you can generate your own on a per COM object basis if you need to be more descriptive.

Unfortunately, AddRef() and Release() are different. Way back before DCOM was a twinkle in the eye, in the time when Inside OLE was the COM bible, it was decided that AddRef() and Release() would return the updated reference count. It probably seemed like a good idea at the time... Once proxies and DCOM came on the scene it was no longer possible for the COM runtime to guarantee that the return value from AddRef() or Release() was in any way related to the actual reference count of the object; proxys might optimise the passing on of AddRef() and Release() calls to the stub and so the object could never really know how many client references were held and neither could the proxys. The end result was that AddRef() and Release() are amongst the few COM interface methods that don't return an HRESULT and the value that they do return is, generally, useless.

The history lesson is just so that we can discount the usual method of indicating failure from a COM object. We can't simply return E_FAIL or some other failure code to tell the caller that there's a problem. So what can we do?

As usual, it depends. From the snippet of code and commentary in Raymond's posting we can't tell exactly how this COM object is likely to be used. The reason that he's "at least" adding an assert is that the object in question has a bug that means that the reference count of the object is being reduced to 0, at which point the object destroys itself and during the destruction process the object uses an interface on itself which causes AddRef() to be called and the subsequent call to Release() means that the object is destroyed twice... The assert is there to catch the bug rather than waiting for it to show up as memory heap corruption or invalid pointer use.

The reasons that I have stated for disliking the use of assert in C++ still apply to this piece of code:


The situation is somewhat more constrained than "normal" C++ code. We can't change the design, by having AddRef() return an HRESULT, perhaps, as we don't own the interface. We're on a code boundary between C++ and the COM runtime; if we throw an exception then we're breaking some rules, the exception will cause problems if the object is remoted, and defending that position is harder than if we weren't at a COM border. The actual problem is happening from within a C++ destructor; this is another boundary where it's not wise to allow a C++ exception to cross. So, what do we do?

Once again, it depends. In this particular situation I'd probably just fix the bug in the destructor; the simplest fix is for the destructor to increase the reference count to one before doing anything that may result in AddRef() or Release() being called; in the absence of other reference counting bugs the last external reference being released will decrement the reference count to 0 and initiate destruction, once inside the destructor any additional calls to Release() will only ever bring the count back down to 1 and so avoid double destruction. This will prevent the double destruction problem that the code currently exhibits. I'll ignore the question of whether it's wise to be doing all of that work in the destructor.

Of course that doesn't change the fact that AddRef() and Release() have some unchecked preconditions. Shouldn't we use assert for them?

Not if I can help it, no. Often, when you're using ATL for example, you don't own the AddRef() and Release() code (and ATL uses asserts anyway!) and, to be honest, I've not often been in the position where COM reference counting bugs were such a problem that I've needed to deal with these preconditions; usually you can simply deal with the obvious issues in the code that uses the COM object... But, if I did own the code, and it was a problem, or I felt I needed to prevent it ever being a problem, then I would try not to have to use the traditional assert for two of the reasons stated above; debug only checking and difficulty in regression testing.

First I'd investigate exceptions. If the object in question is only ever going to be used in process and never across apartment (and we can should these things impossible if that were the case) and the problem is something that we're having a lot of regressions on and therefore we need a reliable test for it in future then I'd use an exception, and have the tests ensure that the object never throws. If the object can legitimately be remoted then things are harder. Given that the standard assert either dumps you into a debugger or terminates the program there and then I don't really see that that's any less severe than breaking the COM rules, but we're on a slippery slope. We could do some investigation work; if we were to validate the preconditions with a test that throws an exception then the COM runtime would likely deal with it in the same way that it would if we were to touch memory that we shouldn't have touched. The stub is likely wrapping its call to our object in a Windows structured exception handling block and would return an RPC_xxx error to the proxy (remote interface AddRef() and Release() are dealt with by the proxy and stub using IRemUnknown and the RemAddRef() and RemRelease() methods do return HRESULT error codes). Of course I'd investigate a little more and test this theory before relying on it. If it works then out of process objects could then be tested because failure would mean the failure would be reported by the COM runtime. In process objects could be tested because a test of an in process COM object, in C++, can test to ensure that the object doesn't break the rules of COM by throwing exceptions across interface boundaries. If this worked then the code could stay in both production and development builds... There are a lot of assumptions there and we're breaking rules, I'm not convinced that the result is worth it, but, as always, it depends.

If this didn't work, or if I was unable to justify the design to my peers, and both are equally likely, then we could investigate a non standard form of assert. This would need to remain in production builds and report its failure in such a way that it could be monitored from a test harness thereby making assertion failure use a programatic error reporting channel rather than the human channel that it usually uses. If the problem wasn't considered important enough to require the extended development but it was serious enough for us to want to prevent it then I'd probably suggest an more standard assertion that remained in the production build. If we profiled the system and found that wasn't acceptable from a performance point of view then I'd argue my point and hope that someone else could come up with an acceptable solution. If they didn't, I expect I'd use a standard assert. The key point is that it should always be the last resort, and as such you should always try and solve the real problems first.

Thanks Mark, it was an interesting journey.

[Updated 3rd October]
See the following follow up posting:

  • Asserts are evil, except when you have no other choice
  • 1 TrackBack

    Asserting Oneself from Games from Within on October 3, 2005 4:28 AM

    I've been following the discussion on the evils of assert started by Len Holgate. Poor old assert was getting beaten up from every side, so I'm going to have to step forward and defend it. Read More

    16 Comments

    > The reasons that I have stated for disliking the use of assert in C++
    > still apply to this piece of code:

    > The check will, likely, go away in a release build.

    Invalid argument. Just use an assert that stays in the release build.
    (you suggest this yourself later)

    The key point here is *control* and that in different scenarios you want
    different things to happen when triggering an assert. In the nightly
    build you do not want a dialog box that blocks testing, but some kind of
    logging (an email?). In interactive use you want the dialog box. When I
    am writing a unit test, I want it printed to the console so my editor
    can jump to the file with the error. If it is code running in a server
    scenario you would likely want to attempt some form of recovery or even
    restucture your code so that asserts are removed and "handled" as errors
    in the code. In my view the latter can be incredibly difficult to do in
    practice.

    Even if an assert is only present in release mode, it still has a value
    in debug mode and therefore asserts are useful and not evil.

    > The assert is covering for poor design.

    asserts are some of the least likely constructs to affect design. It is
    limited to a single line of code.

    > The assert makes it very hard to write a unit test for the failure
    condition.

    > If the object is running without a user interface then the assert's
    dialog box may cause issues.

    Again, use an assert that does what you want.

    For example, my unit test framework catches exception so sometimes I use
    an assert defined like this

    #define ERRPOS __FILE__, __LINE__
    #define MIL_ASSERT(expr) if (!(expr)) throw CmMilEx(ERRPOS, "ASSERT " #expr);

    // (CmMilEx is my base exception class based on a std::exception)

    This way asserts are an extension of unit testing.

    I am regretting that my "assert" exception inherits from
    std::exception. It means that my program errors can be hidden if someone
    uses a "catch (std::exception)" !. Scary. In C++ I can make a parallel
    hierarchy and only fear people who uses catch (...) incorrectly. Sadly
    this is not possible in C# where it is needed more (in my limited
    experience).

    > and finally: The assert is really just an "exploding comment".

    Not an argument (just name-calling).

    >
    > ...
    >
    > If this didn't work, or if I was unable to justify the design to my
    > peers, and both are equally likely, then we could investigate a non
    > standard form of assert.

    So asserts are not evil after all.

    Will you post a retraction? :-)


    I do not hope I come of as some mad assertion sprinkler. I am interested
    in good guidelines for when to use asserts and when to use
    exceptions. The most thoughtful analysis I have seen so far is Herb
    Sutters' "When and How to Use Exceptions" in C/C++ Users Journal August,
    2004.

    After rereading it (and understanding more the second time) my focus is
    currently on these comp.lang.c++.moderated messages

    http://groups.google.com/group/comp.lang.c++.moderated/browse_frm/thread/d2eb226637b80e3d/7ec9dc586af71a9a#7ec9dc586af71a9a

    http://groups.google.com/group/comp.lang.c++.moderated/msg/41e5fee6f337108e
    ("Programmer errors" vs "Exceptional conditions")

    > > The assert is really just an "exploding comment".

    > Not an argument (just name-calling).

    Well I still use C over C++, so I might get a pass on using evilness so much :) ... but I didn't take that as namecalling but as a compliment. asserts are like comments that are checked at runtime, so you know they are correct ... exploding if they are wrong.

    Saying that I'd clasify "an assert" as a pattern, if you do a check and throw an exception instead of calling abort() or name it BUG_ON() and reboot the machine, that's still an assert ... and if you are typing more than MY_ASSERT(test, exception) ... then you are just wasting space and programer time.

    "a compliment"? Len was listing bad things about asserts, so I understood it as something negative.

    > I'd clasify "an assert" as a pattern,

    Yes. That is exactly my point.

    Handling of an assertion failure is implementation specific. The fact that the standard C assert is compiled away in release mode is irrelevant to the idea of an assert (Len even mentions a usage scenario were this is valid).

    > .. and if you are typing more than MY_ASSERT(test, exception) ... then you are just wasting space and programer time.

    I don't understand this comment. What more could imagine typing?

    VJ,

    > > The check will, likely, go away in a release build.

    > Invalid argument. Just use an assert that stays in the release build.

    As I've been saying all along, I have issues with the use of the traditional C style assert in C++. The traditional assert usually compiles out of release builds. I have an issue with this.

    > The key point here is *control* and that in different scenarios you want different things to happen when triggering an assert.

    Non standard asserts are less of a problem but there's still the issue of coupling. The more complex the replacement assert the more every piece of code in the system is coupled to. Sure you can write an 'assert' that deals with all of my issues with the traditional assert but in doing so you're coupling all code that uses your new assert and all code that uses that code to it. That may, or may not, be appropriate. To that end I'd prefer to use a solution which requires less coupling. Exceptions require less coupling because the class that throws them can decide what to throw. It's not coupled to anything else. The calling code is already coupled to the throwing code and the exception that can be thrown is just a part of the throwing code's interface so no additional coupling is incurred.

    Contrast the above to the situation where you have a specialised assert which you use. All the code that uses it is now coupled to that single thing; if you change it you must "rebuild the world". If you wish to use the code you need to understand this new assertion, how it works, how it transmits details of failure, etc. You're creating a barrier to the adoption of your code. This may or may not be a problem to you.

    > it is code running in a server scenario you would likely want to attempt some form of recovery or even restucture your code so that asserts are removed and "handled" as errors in the code.

    Indeed.

    > In my view the latter can be incredibly difficult to do in practice.

    Not if you use exceptions.

    > Even if an assert is only present in release mode, it still has a value in debug mode and therefore asserts are useful and not evil.

    I assume you mean "is not present". My point is, and has been all along, that their use is an "quick and easy fix" and not a correct fix.

    > asserts are some of the least likely constructs to affect design. It is limited to a single line of code

    I disagree. Every time you think "this shouldn't happen, we'll stick an assert in here" you're going for the easy fix rather than fixing the real problem. If you must have elements in the list before calling pop() then it's an error to call pop() without any elements. You have two options, either you ignore it and allow the list to exhibit undefined behaviour (I'll get back to that later) or you prevent it and keep the list valid. Given that in C++ it's preferable to use the return value for the thing you're popping so that the caller can assign it to a const if they wish and given that in code that uses the list correctly the return value of pop() is never optional it's inappropriate to return a pointer which can be null if the list is empty; doing so complicates the 99% case at the expense of the 1% inappropriate use case. In this case you only have one standard error channel left and that is the use of an exception to report the failure condition. You may say that placing an assert here is equivalent, it's not. The assert will use a non standard error reporting channel, be that a dialog box or whatever clever channel that you come up with. As soon as you use that channel all code using your code is coupled to that channel. If the assert goes away in release builds then you are basically saying that you don't care about potential undefined behaviour in release builds... Now all of the anti-exception zealots will leap in with "what if it is valid to try and pop an empty list", or "what if we need an atomic possibility test and pop", if these are an issue for you then provide another function PopIfPossible() ? This can returns a pointer that can be null if the list is empty... The code that deals with this case can then react accordingly.

    Going back to the potentially undefined behaviour thing. I notice from Dave Abrahams message that you linked to that he defines "programmer errors" as something that leads to precondition failures which lead to undefined behaviour and "exceptional conditions" which are checked for in the code and lead to exceptions. I just don't buy the distinction. As I said before, I don't think I live in the same world as Dave. My aim is to write code that works and is bug free. Writing code that works is hard. I don't want to make it harder by deliberately allowing programmer error (which I find is extremely common) to lead to undefined behaviour (which is often very difficult to diagnose and fix). All that doing so can do is lengthen development time and confuse the programmers who need to use your code. To that end I want to catch programmer error in the same way that I want to catch exceptional situations. I do not have any intention of deliberately choosing to allow undefined behaviour.

    > Again, use an assert that does what you want.

    I refer you to my coupling argument above...

    > For example, my unit test framework catches exception so sometimes I use an assert defined like this

    I have problems with this, too ;) though not that many... Personally I don't find it that useful these days to include file and line information. I can understand how you might have some way of getting your IDE to navigate automatically based on program output, but, I'd personally prefer the exception message to just include the class name, method and the reason for failure. The reason for my preference is subtle. If you structure your code in such a way that it's easy to locate the throw site from the information that I prefer to provide then the code is easy to navigate. If you provide a file and line number then you're effectively admitting that it's hard to navigate your source tree... I don't find line numbers that useful because I keep most of my files pretty small... I'd also prefer to have a human readable message rather than just the textual representation of the failing test. This requirement precludes the use of the stringize operator (#)... So, rather than my exception saying "i > size()" I would rather it said "index out of range: 15 (max is: 10)". These requirements remove the need and preclude the use of a 'clever' macro... This leads me to the test becoming:

    if (testFails)
    {
    throw myException("location", "problem");
    }

    This has the advantage that it's normal code (more on that later) and that it communicates clearly. The disadvantage is that it's more than just a one line macro and requires more typing (this is what James was inferring). I don't have a problem with the disadvantage because I'd rather spend a little more time and thought whilst programming because I know it saves me time when I'm using or maintaining the code...

    The "Exploding comment" isn't just "name calling" but it also isn't that much of a compliment either. I'm a believer that comments shouldn't be required that often in properly designed code. Most of what comments offer can be achieved with correct naming and correct factoring of code into functions. The advantage of an assert over a comment is that the assert can't rot because it gets executed at least sometimes. The disadvantage is that it's still not just normal code...

    The differentiation between assert and normal control flow code is a problem for me. By separating error checking from normal code by using a different style for it you risk making it seem "less important" than normal code; more so given that the standard assert goes away in production code. It sends the wrong message. Error checking is important. It's part of the "real" code. It's as important (if not more so) as the "real" code. Given the demonstrated confusion over what constitutes an assert in C++ it's easier, and more precise, to say "don't do it!". Yes you should check input parameters. Yes you should validate returned data. Yes you should make sure objects don't get into inconsistent states. No you shouldn't use assert for this. Just use normal code... It's error handling. It's part of the normal work you do. It's not special.

    So: Assert is evil. It's a tool that is still in my toolbox. It's a tool that I do still occasionally use. It's a tool that I'll always try and avoid using by finding a better way to achieve what I'm trying to do. It's not a tool that I encourage the use of...

    I don't think you're mad VJ; I hope you don't think I'm some assertion removing zealot.

    Unfortunately I don't have the August 2004 C/C++ Users Journal to hand; I have limited storage space and I had a bit of a purge before my ski trip in January... Do you know if it's online at all, or perhaps incorporated into one of his books?

    James,

    I C I agree that assert can be a pattern... In C you don't have an alternative error channel (I'm not going to suggest the use of longjump ;) ). So the whole assertion pattern thing is more valid.

    In C++ I'd almost call assertions an anti-pattern... Mainly for the reasons stated in my response to VJ. Coupling, unnecessary additional error channel, differentiation from "normal code", etc.

    Again, I disagree re the ideal that an assert that's any more than a single line is wasting programmer time, see my response to VJ re communication... And lets not get me started on macros, eh ;)

    Len, I disagree :)

    1. I don't consider assert() usage, no matter what happens when it fails, an error checking path. If you are going to do different things depending on where the "assert" failed, it's normal error checking and you should propagate that as an explicit code path (return values, checked exceptions or just documented exceptions).

    Assert is for those times when X _has_ to be true, if it isn't something bad happened that can't be compensated for. For instance, I'll often write loops like:

    int i = 0;
    while (i < abcd)
    {
    ++i;
    /* stuff */
    }
    ASSERT(i == abcd);

    ...this is not recoverable, if the assert is triggered it means that I screwed up inside the loop and that needs to be fixed.

    2. Given that you are always going to fail in the same kind of way (be it via. abort(), an infinite loop or exceptions) it seems obvious to me given:

    ASSERT(i == abcd);

    ...and...

    if (i != abcd)
    {
    throw assert_exception("i isn't abcd", "foo");
    }

    ...the 4x increase in number of lines is not doing anything useful, is making it less likely the programer will write it in the first place and is providing noise when trying to read it later.

    3. As for compiling them out, personally I do but I can see the other POV ... my main objection is that I do a _lot_ of validation inside functions like:

    void obj_waiting2process(Obj *obj)
    {
    ASSERT(obj__valid(obj));

    que_del(&q_io_waiting, obj->que_io);
    que_add(&q_io_process, obj->que_io);
    obj->flag_q_io_waiting = FALSE;

    ASSERT(obj__valid(obj));
    }

    ...where obj__valid() will walk the q_io_waiting list and the q_io_process list making sure that obj is on one, and not the other depending on what the value of flag_q_io_waiting is (as well as all the other things inside obj).

    Now, I may not be a genius ... but I'm pretty sure having O(1) algorithums be O(2n) is not such a hot idea :).

    Sure there are cases like (i == abcd) where it'd probably not affect the production binary, but I'd rather not have "fast" and "slow" asserts and making sure the asserts have been run and passed in debugging is what the unit tests are there for IMO or to put it another way ... if the assert isn't being run in debugging, how do you know the assert isn't wrong (Ie. in the above case maybe there is a corner case where the loop should be broken before i == abcd, and the code works fine).
    The solution here is to test that code path in the unit tests, not make debugging the production code mildly easier :).

    James

    I wouldn't use an exception to check that loop construct. I also wouldnt use exceptions to validate your object state. I wouldn't use asserts either. I'd aim for being able to test these things externally to the code.

    I realise that sometiems it seems difficult to test these kinds of things from outside the code that's doing the actual work but that's where Test Driven Development helps. If you start with the tests that you want to run then your design is pushed in the direction that makes it possible to write those tests.

    So, in the case of all of your examples I'd hope to have tests outside the actual production code that proves that the production code works. I suppose you could look at these tests as "compile out" assertions as they're not present in the production code itself...

    Just a quick post about the Sutter article.

    From a Herb Sutter usenet posting:

    "Several responders already noted my article "When and How to Use
    Exceptions" (C/C++ Users Journal, 22(8), August 2004). It's drawn from
    Items 70 to 72 of C++ Coding Standards
    (www.gotw.ca/publications/c++cs.htm). "

    I don't have it (yet) but I would be very interested in reading item 68.

    I will post more later when have time :-) (it's my birthday today and I am starting on a new job on monday)

    Thanks, I'll read it later.

    Happy birthday!

    I've just read items 68 through 75 of Sutters' C++ Coding Standards and I agree with all of them except 68 :) I think I could agree with 68 if I'm allowed to read it the way I'd like to... So let's see.

    Item 68 is "Assert Liberally to document internal assumptions and invariants". The interesting thing about it is that it starts off by stating that you should use assert to document assumptions internal to the module and then defines a module as being "where the caller and callee are maintained by the same person or team". That's an interesting definition. The first thing I'd do is change that definition so that it's a little more precise. After all, at present it could mean all of the code that I write for my clients and I don't feel that's quite right... Personally I'd prefer the definition of a module to be a library. After all it's a reasonable grouping of functionality and there's no telling when ownership or maintenance responsibilities for other libraries or bodies of code that call into the library will change. So lets stick with the spirit of the original definition and say that asserts are only appropriate for validating internal assumptions and invariants in a library. That implies that at the border of the library, i.e. the publicly facing API, you should be validating incoming arguments and outgoing results and generally reporting errors using some other method. In this scheme I would assume that a user of the module should never be able to trigger an assertion failure with bad data... Of course you may opt not to check input in some situations (I'm thinking about performance requirements and how the STL std::vector doesn't check the index supplied to operator[] but, ideally, these situations are rare.).

    So, we have a library with a public API which uses an error checking policy and calls internal to the library that use assertions to report programmer error. With this view, for example, if the API takes a pointer, the public layer validates that pointer is non null and the internal code assumes it is non null and uses assertions to enforce that assumption... I'm not especially comfortable with that idea for all of the reasons that I've stated before. Taking that approach may encourage me not to adjust the design so that the problem can't happen (pass by reference internally even if you must accept a pointer in the public API). It may encourage me to only test from the public API, as triggering the assertion may make it difficult to test the internal object in isolation. If a design change means that need to expose the internal object we need to go and look at the object and replace assertions with error checking that's appropriate for the public API... We effectively have two kinds of object, public ones and internal ones... So, in this case, as I've said before, I'd prefer to consider assert as the last option rather than assuming it's the first option and liberally sprinkling them in the code.

    Next, item 68 goes on to suggest supplying your own assert, retaining the checks in production builds and making everything more complex with multiple 'levels' of checking and reporting (hmm); broken, really broken and totally really honestly very broken, perhaps? ;)

    It then goes on to suggest using assertions to check for sane values of variables inside your classes. The example given is of an employee ID which can never be 0. At the point where the class uses the employee ID it has an assertion to ensure the id is never 0. I disagree. The requirement that the employee ID is non zero is, no doubt, enforced by some form of permanent test that occurs at the point where the value is changed. Ideally the value is only changed in one place, so we have one place to ensure that the value cannot become zero. If the value never changes then, ideally, it's checked and set in the constructor and it's stored in a member variable that is const. I would consider use of assertions to validate this situation at every point of use to be adding noise to the code. The place to enforce the rules around the value of a variable is at the point where that value can change. If you ensure that you enforce those rules at that point then adding asserts at the point of use seems to simply add confusion. It seems that you're saying "I'm not sure if something could have changed this value without enforcing the conditions attached". I'd rather be sure and remove the ambiguity. I spend a lot of time reducing ambiguity in code; correctly using const, reducing the scope of variables, removing pointers and using references, etc. The result is that you can look at the code and be sure. I'm not assuming about the code, I know. I can see that if you don't know, or you're not convinced, then an assertion may help you. I'd prefer that you know and that you adopt habits that mean you DO know. I see the use of asserts as a crutch that prevents you adopting that habits that mean that you will know.

    I realise that almost all of these pro assert examples are trivial. I'd quite like to see a non trivial example and see how my approach compares.

    Just when I thought that I'd extricated myself from the assertion debate - Len produces a comment that just begs for refutation :-)

    Mark,

    Hmm, I feel a little misrepresented ;) but you have a point. Unit tests have the marvelous effect of keeping you well aware of your limitations.

    Just as I don't expect other programmers to dumb down code with pointless comments i++ // increment i just in case someone who doesn't understand the language wants to read it, I dont expect to have to write code with unit tests and then add a bunch of assertions for invalid object states that the unit tests will catch. If, under maintenance, the code breaks because the unit tests weren't run then there's a programmer problem, before there's a programming problem.

    I stand by my original statement; traditional assertions are evil. And my later suggestion: use them only as a last resort.

    More, later, no doubt, this one seems to just run and run...

    The other thing that I forgot to mention last night is that if you do apply some simple transformations to the code which eliminate the need to assert precondition then the regression or bug that you're trying to guard against no longer exist.

    I've updated my article to show your comment.

    We use asserts that only appear in debug builds. If you have no problem with these kind of asserts then feel free to ignore this. The only person who sees the debug builds are the programmers. Since our test harnesses are run over the code during a daily build, the programmers need all the sanity checks they can get to make sure the changes they have made for that debug session haven't seriously broken something. Asserts ARE a problem in loops and threading stuff where they can cause havok - but these are easily commented out during debugging. I'd rather my fellow programmers add them, so I can temporarily comment them out rather than risking the situation where the absense of an assert has wasted you stepping thru invalid state code for half an hour. They save valuable debugging time, and when you realise that is where MUCH programmer time is spent they are of obvious benefit.

    >The check will, likely, go away in a release >build.
    Only valid complaint if the assert is in lieu of release build checks. If you have a test harness, and a corresponding test also exists there, then no problem.

    >The assert is covering for poor design.
    Only valid if the assert is in lieu of poor design. Again it doesn't have to be.

    > The assert makes it very hard to write a unit > test for the failure condition.
    Not if it goes away in release.

    >If the object is running without a user >interface then the assert's dialog box may cause >issues.
    valid, but easily commented out by the programmer debugging it.

    >and finally: The assert is really just >an "exploding comment".

    Comments are good!?!?

    Oh good, here we go again ;)

    Anthony, your assesment of my reasons for disliking asserts is pretty accurate. The thing is that the way many people view asserts causes my complaints about them to be valid in much of the code that I get to see on client sites.

    I actually quite like the implied suggestion that since your asserts go away in release builds and they dont replace required checks and you have unit tests that cover the error situation that the assert covers then they're not a problem because you can run the unit tests on the release build and get the "value" of the assert in the debug build. However... I'd probably prefer to spend less time in the debugger in the debug build (where the asserts can help me) and run the test suite on it instead. But that's just me.

    As for "Comments are good!?!?" my view is that yes, in certain circumstances comments are good, but those circumstances are few and far between. Comments are good if you can't clearly communicate the same information in code that compiles.

    Leave a comment

    About this Entry

    Book review: Rootkits by Hoglund and Butler was the previous entry in this blog.

    Once again I've been too busy to comment on these during the week is the next entry in this blog.

    I usually write about C++ development on Windows platforms, but I often ramble on about other less technical stuff...

    Find recent content on the main index or look in the archives to find all content.

    I have other blogs...

    Subscribe to feed The Server Framework - high performance server development
    Subscribe to feed Lock Explorer - deadlock detection and multi-threaded performance tools
    Subscribe to feed l'Hexapod - embedded electronics and robotics
    Subscribe to feed MegèveSki - skiing