[Insight-developers] ExceptionObject Borland Compile errors

[Niels Dekker]

Luis Ibanez wrote:
> Please feel free to fix the typo:
>    ExceptionObject::SetLocation( description )

Done:
www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkExceptionObject.cxx?root=Insight&r1=1.13&r2=1.14

I only did some simple local tests; I think that should be sufficient, 
because the fix is so straight-forward.  But I won't make any other 
changes before having submitted another Experimental build.  Which takes 
me quite a few hours, unfortunately...  (So it won't be today anymore.)

> and to make the improvement of adding a local
> variable to avoid calling GetExceptionData()
> multiple times.

Thanks  :-)  As I said, it will take some more time, but it isn't as 
urgent as the typo found by Karthik.

> BTW: please note that the c_str() calls *are* needed
> by Borland, so please keep those ones in place.

I do understand that you'd rather not want any more Borland errors for a 
while. But I still wonder why Borland would need the c_str() calls.  I 
thought it was because the operands at the right of the 
question-mark-operator had a different type, in my original version:

  IsNull ? "" : m_ExceptionData->m_File

In such cases, my copy of Borland 5.5.1 appears to say "Error E2354: Two 
operands must evaluate to the same type".  Apparently because "" is a 
char-pointer, while m_File is an std::string.  This issue could be 
fixed, simply by having an std::string for both operands, e.g.

  IsNull ? std::string() : m_ExceptionData->m_File

In the new version, it would look more like this:

  thisData ? thisData->m_File : std::string()

Anyway, if you want the c_str() in there, no problem, but I don't think 
it's really necessary...

> I would rather keep the dynamic_cast<> because
> when the pointer is not of the appropriate type,
> at least a NULL pointer will be produced, while
> the static_cast will convert the pointer even
> if no conversion is valid.

Okay, in that case I think it's just a matter of taste. I would do 
static_cast, to state explicitly that the pointer is in fact an 
ExceptionData pointer. Because it's hard-coded that way. While you'd 
rather double-check, just to be sure.  Which definitely makes sense as 
well, so I'll leave the dynamic_cast in there.

> Do you see any drawback in the use of dynamic_cast ?

dynamic_cast is known to have some performance penalty.  But I don't 
expect it to be very large, in this particular case. Anyway, there's 
more about the performance of dynamic_cast in the Technical Report on 
C++ Performance: www.research.att.com/~bs/performanceTR.pdf

> Changing the return type of GetExceptionData()
> sounds fine.

Thanks  :-)

> Also, If you have a chance to verify that this is
> still solving the original issue related to crashes
> at the moment of throwing exceptions, that will be
> great.

Good point. I should think about how to unit-test it!

Good night (good nightly build!),

  Niels