[Insight-developers] STYLE: Un-necessary use of static_cast -- should we care?

[Matt McCormick]

On Mon, Jul 23, 2012 at 8:42 PM, Williams, Norman K
<norman-k-williams at uiowa.edu> wrote:
> Hans assigned me this ITK ticket to look into things:
>
> https://itk.icts.uiowa.edu/jira/browse/ITK-2950
>
>
> His feeling was that there were cases where dynamic_cast and checking the
> returned pointer for 0 would be a more robust solution.
>
> So I did an exhaustic examination of the use of static_cast in ITK.  This
> amounted to 1408 (or more, sometimes they show up twice in the same line)
> occurrences in ITK.
>
> I pared this down to about half that many by eliminating casts to
> intergral types, pointers to integral types. etc.
>
> What I discovered is that there are many cases when static_cast is used
> unnecessarily.  The most common being this idiom:
>
> DataObject::Pointer returnDP()
> {
>   SomeDataObject::Pointer someData = SomeDataObject::New();
>   return static_cast<DataObject *>(someData.GetPointer());
> }
>
> As SomeDataObject is always a class derived from DataObject, the return
> value of someData.GetPointer() is assignment compatible with DataObject *
>
> The only reason I can see to keep this usage is the idea that the
> static_cast is for human eyes, not the compiler.
>
> There's also a converse case, where dynamic_cast might do something useful:
>
> template< class TOutputMesh >
> typename MeshSource< TOutputMesh >::OutputMeshType *
> MeshSource< TOutputMesh >
> ::GetOutput(void)
> {
>   return static_cast< TOutputMesh * >( this->GetPrimaryOutput() );
> }
>
> This assumes that the PrimaryOutput will in fact be of type TOutputMesh.
> This may always be the case, but only because of programming discipline.
> Both for GetInput and GetOutput, some faith is being placed in the filter
> writers, but it isn't verified by compile-time type checking.
>
> In those cases I can imagine that dynamic_cast + checking the returned
> pointer might be a safer way to go.  On the other hand I just spent
> several days dealing with the overhead of GetInput and GetOutput, and this
> would only add to it.
>
> So the question is this: do we keep static_cast where they aren't needed?

Since it is a matter of human-readability, something consistent is
preferred, whatever that may be...

> and should we add dynamic_cast in the frequent case where an input or
> output of a filter is assumed to be of a particular type?
>

If it is not logically possible in the code for the dynamic_cast to
fail, and given the GetInput/GetOutput consequences, avoiding
unnecessary overhead seems best.

Thanks,
Matt

>
>
>
>
> --
> Kent Williams norman-k-williams at uiowa.edu
>
>
>
>
>
>
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
> ________________________________
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.php
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers