[Insight-developers] Templated itk::Math floating point to integer proposal

[Tom Vercauteren]

Hi Brad,

Thanks for the feedback, this is exactly what I am asking for.


> 1) replacing
>  int  itk::Math::Round( double x )
> by
>  template <typename TReturn>
>  TReturn itk::Math::Round( double x)
> would break backward compatibility
>
> Can the template and non-template version co-exist? Isn't this just a form
> of function overloading? (my gcc compiler will allow this)
>
> Do we have to replace?
>

You are right. Actually I didn't realize that since the template parameters
also participate to the function signature, this kind of overloading should
work.


My current approach is to leave the current functions as is and add a
templated version of them. They are prepended by a capital T, e.g.
 template <typename TReturn>
 TReturn itk::Math::TRound( double x)


I would still think that we should surround the old Round method with
> "#ifndef ITK_LEGACY_REMOVE" because this will help to track down all usages,
> and make sure we get the right type. It will also prevent the wrong methods
> from being used again.
>

Right, this has been changed in the new version of the patch.

2) I have added a itk::Math::Detail namespace within itk::Math to put
> helper functions that should not be used by end-users but need to be
> in the header file to be potentially inlined
>
>
> 3) I chose not to directly rely on the functions from vnl_math.h to
> ease the process of writting and modifying these functions
>
>
>
> It would be nice if we could add that "Detail" namespaces are excluded from
> Backwards compatibility policy.
>

I do agree. Could we add such a rule to ITK's backward compatibility policy?

Another option would be to not use a it::Math namespace but rather create a
itk::Math class that only has static member function. This allows for
putting the helper functions in the private section. In that way we ensure
that nobody can access them and hence that these helper function do not need
to follow ITK's backward compatibility policy.

e.g.

class Math {
public:
   template <typename TReturn, typename TInput>
   static inline TReturn Round(TInput x)
   {
      if (sizeof(TReturn) <= 4)
      {
         return static_cast<TReturn>(rnd_32(x));
      }
      else if (sizeof(TReturn) <= 8)
      {
         return static_cast<TReturn>(rnd_64(x));
      }
      else
      {
         return rnd_base<TReturn,TInput>(x);
      }
   }
private:
   static inline Int64 rnd_64(double x){ ... }
   static inline Int64 rnd_64(float  x){ ... }
   static inline int rnd_32(double x){ ... }
   static inline int rnd_32(float  x){ ... }

   template <typename TReturn, typename TInput>
   static inline TReturn rnd_base(TInput x)
   { ... }
};

However having a Math class with static functions only seems rather ugly and
not scalable to me.



>
> Also, I am not sure if it's still time to commit something like that.
> There is no release schedule for 3.16
>  http://www.vtk.org/Wiki/ITK_Release_Schedule
> But if we are close to the feature freeze, I'll definitely hold on
> before moving on.
>
>
> Luis?
>

Luis: Thank you for your wiki update.

Regards,
Tom
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090730/2506b0e5/attachment.htm>