[Insight-developers] ExceptionObject Borland Compile errors

[Luis Ibanez]

Hi Niels,

Please feel free to fix the typo:

    ExceptionObject::SetLocation( description )

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


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


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.

Do you see any drawback in the use of dynamic_cast ?


Changing the return type of GetExceptionData() sounds
fine. I don't see any advantage or disanvantage either
way.


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.


Sumbitting Experimental builds will also be great...  :-)


    Thanks


        Luis



---------------------
Niels Dekker wrote:
> Karthik Krishnan wrote:
> 
>> Also line 255 should read
>>  ExceptionObject::SetDescription( description );
>> instead of
>>  ExceptionObject::SetLocation( description );
> 
> 
> Oops!  Luis, are you gonna fix it, or do you want me to?
> 
> Two small PERF/STYLE thingies:
> 
> The SetLocation and SetDescription overloads for std::string could avoid 
> having multiple GetExceptionData() calls, as follows:
> 
>  void
>  ExceptionObject::SetLocation(const std::string& s)
>  {
>    const ExceptionData * const thisData = this->GetExceptionData();
> 
>    m_ExceptionData = ReferenceCountedExceptionData::ConstNew(
>      thisData ? thisData->m_File : std::string(),
>      thisData ? thisData->m_Line : 0,
>      thisData ? thisData->m_Description : std::string(),
>      s);
>  }
> 
>  void
>  ExceptionObject::SetDescription(const std::string& s)
>  {
>    const ExceptionData * const thisData = this->GetExceptionData();
> 
>    m_ExceptionData = ReferenceCountedExceptionData::ConstNew(
>      thisData ? thisData->m_File : std::string(),
>      thisData ? thisData->m_Line : 0,
>      s,
>      thisData ? thisData->m_Location : std::string());
>  }
> 
> 
> GetExceptionData() could return an ExceptionData pointer, instead of a 
> ReferenceCountedExceptionData pointer (and it could do static_cast for 
> PERF reasons, as I wrote before):
> 
>  const ExceptionObject::ExceptionData *
>  ExceptionObject::GetExceptionData() const
>  {
>    const ExceptionData * thisData =
>      static_cast< const ExceptionData *>(
>        this->m_ExceptionData.GetPointer() );
>    return thisData;
>  }
> 
> This looks more elegant to me, because this cast is more limited (from 
> ReferenceCounterInterface to its directly derived class).  And because a 
> ExceptionData pointer is sufficient anyway, whenever GetExceptionData() 
> is called.
> 
> 
> What do you think?
> 
> 
> Kind regards,
> 
>  Niels
>