Re: [lmi] Safe and consistent dereferencing and casting [Was: Code review: product editor]

[Vadim Zeitlin]

On Fri, 23 Mar 2007 22:59:33 +0000 Greg Chicares <address@hidden> wrote:

GC> so I think I should change the body at least this much:
GC> 
GC>   wxWindow* f = GetFrame();
GC>   LMI_ASSERT(f);
GC>   return dynamic_cast<wxFrame&>(*f);
GC> 
GC> And you make a case for something like this:
GC> 
GC>   wxWindow* w = GetFrame();
GC>   LMI_ASSERT(w);
GC>   wxFrame* f = dynamic_cast<wxFrame&>(*w);
GC>   LMI_ASSERT(f);
GC>   return *f;
GC> 
GC> in order to get __FILE__ and __LINE__ if the cast fails. That
GC> also tells us whether the pointer was null originally, or the
GC> type was wrong.
GC> 
GC> Copying a one-line idiom is okay, but copying a five-line idiom
GC> ten times doesn't sound right. Here's a different idea:
GC> 
GC> -    return dynamic_cast<wxFrame&>(*GetFrame());
GC> +    return safely_dereference_as<wxFrame>(GetFrame());

 Hello,

 First I'd like to note that by using a function here you won't be able to
get the file name and line number of the place where the cast failed. Of
course, it's not a big problem as there is a standard solution: just define
a macro which would call this safely_dereference_as function with __FILE__
and __LINE__ parameters which it would then use to construct the error
message if the cast fails.

 Anyhow, in spite of this small problem I still think that implementing
this function would be a good idea we've just had quite a discussion with
Evgeniy about how exactly should it be implemented. This matters because
there are other similar wrapper functions which we need (notably the
function for safely dereferencing a boost::any object) and it makes sense
that they should be implemented in a similar way so it would be great if we
could agree on The One True way.

 Basically there are 2 options for implementing this function:

        template <typename T, typename U>
        T& safely_dereference_as(U *ptr)
        {
                try
                {
                        if ( ptr )
                                return dynamic_cast<T>(*ptr);
                }
                catch ( std::bad_cast& e )
                {
                        // nothing to do here
                }

                fatal_error() << ... ;
        }

or

        template <typename T, typename U>
        T& safely_dereference_as(U *ptr)
        {
                T *p = dynamic_cast<T *>(ptr);
                if ( !p )
                        fatal_error() << ... ;

                return *p;
        }

I strongly prefer the second form as I don't like provoking an exception
which we then immediately proceed to catch and ignore but Evgeniy thinks
that the first version is more clear (while for me it's exactly the
contrary). What do you think about this?

 Thanks,
VZ