[Insight-developers] My Dashboard Compile Errors and assert()'s

[Gaëtan Lehmann]

Le 11 août 08 à 13:41, Stephen Aylward a écrit :

> Dang - I was hoping to not have to go into more detail...
>
>> When an algorithm is dependant of the content of
>> the manipulated data, it is very difficult to test all the cases.  
>> Keeping
>> the asserts in the code can highlight a problem with new data,  
>> which would
>> have be very difficult to found without them.
>
> This is exactly when we should use an exception instead of an abort.
>

No. The bug in SignedMaurerDistanceMapImageFilter I gave in example  
was in a loop. Adding an exception there would have important  
performance impacts.

>
>> Because it can be quite time consuming (when used in a loop, or to  
>> validate
>> a tree property for exampe) and it exits quite badly (but that's  
>> useful for
>> debugging)
>
> Regarding speed...In most ITK applications, I seriously doubt a check
> for an exception is going to have a notable impact on performance
> relative to other computation costs in ITK.   In the rare, inner-loop
> cases, if you feel the exception test is going to be too
> expensive...leave it out and then document in the .h that the program
> doesn't check but expects (for example) the input to be in a certain
> range.   This is what we've done in the past in ITK.   We shouldn't
> change our policy now.   A abort is not a solution.

Of course it can be a speed problem. The tree property example given  
above is a good example: checking that the values associated to all  
the nodes of a tree made of thousands of nodes have a given property  
can be as long as computing those values.
That's something we won't check in user application because that check  
takes time, and because the code must produce a valid tree.

However, if the user application crash or give unexpected results, the  
developer of that application would be pleased to have some starting  
point to debug, only by turning debug mode on. If its because he  
didn't respect a documented precondition, he can identify a bug on his  
side. Otherwise, he can make a bug report, and claim that ITK is not  
perfect to his client - in that case, it would be true.

And the user will never see the abort, because he only use the program  
built in Release mode.

>
>> Exception would also be a way to do the same job, but it requires to
>> carefully catch exception in ITK code, to not mask the exception  
>> sent by the
>> assert-like method - is it really useful?
>
> Exception are more informative than asserts.


I'm not fully sure, but I think that an exception catched by some code  
and resent after having performed some needed operation would lead to  
the lost of the call stack in the debugger. Only the call stack from  
the second throw would be visible.
Can someone confirm that?

I agree that some check should be implemented with exceptions rather  
than asserts, but I don't think we should try to remove all the  
assert() from ITK. They are both useful and safe when used as they  
should.

BTW, maybe we should have a macro to replace the asserts, when  
possible, and give an explicit message in a concise way - something  
like:

#define itkAssertMacro(cond, x) \
   { \
   if( !(cond) ) \
     { \
     ::itk::OStringStream message; \
     message << "itk::ERROR: " << this->GetNameOfClass() \
             << "(" << this << "): " x; \
     ::itk::ExceptionObject e_(__FILE__, __LINE__,  
message.str().c_str(),ITK_LOCATION); \
     throw e_; /* Explicit naming to work around Intel compiler bug.   
*/ \
     } \
   }


It would give something like that in the code:

   itkAssertMacro( ptr != NULL, << "Input image can't be null." );
   itkAssertMacro( v > 1, << "Distance must be greater than 1." );
   itkAssertMacro( label != m_BackgroundValue, << "Foreground can't  
have the same label than background: " << label );

Regards,

Gaëtan



-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr  http://www.mandriva.org
http://www.itk.org  http://www.clavier-dvorak.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 186 bytes
Desc: This is a digitally signed message part
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20080811/826ace8c/attachment.pgp>