[Insight-developers] Class vs Namespace

Luis Ibanez luis.ibanez at kitware.com
Tue Aug 11 09:52:47 EDT 2009


Hi Tom,

Le'ts not use pointers as the return type.

If we do,
then we have to deal with:

   - NULL pointers
   - Unallocated pointers
   - Deallocated pointers

Let's use references, so we can ensure that the
variable receiving the data actually exists.


     Luis


---------------------------------------
On Tue, Aug 11, 2009 at 8:01 AM, Tom Vercauteren <tom.vercauteren at m4x.org>wrote:

> Hi Brad,
>
> From a totally subjective point of view, I prefer the namespace
> approach. The only issue I still have is about the backward
> compatibility policy. The rounding functions need some helper
> functions. It would be better if those helper functions were not
> subject to any backward compatibility requirement. Here are three
> possibilities I can think of:
>
>
> 1) All namespace approach
> ---------------------------------------
> Advantage: seems clearer to me
> Drawback: No control on the access to th helper functions in Math::Detail
>
> namespace Math
>  {
>  const double p = 3.14;
>
>   namespace Detail
>    {
>    // Things here are excluded from the backward-compatibility policy
>    template <typename TReturn, typename TInput>
>    inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
>    }
>
>  // The second argument is fro msvc 6 compatibility
>  template <typename TReturn, typename TInput>
>  inline TReturn Round(TInput x, TReturn * = 0)
>    {
>    return Detail::RoundHelper<TReturn>(x);
>    }
>  }
>
>
>
> 2) All class approach
> --------------------------------
> Advantage: Helper functions not accessible to the users
> Drawback: Somehow confusing for me to have a class with only static
> members and functions (Also potential compiler optimization loss??)
>
> class Math
> {
> public:
>  static const double pi;
>
>  template <typename TReturn, typename TInput>
>  static inline TReturn Round(TInput x, TReturn * = 0)
>    {
>    return RoundHelper<TReturn>(x);
>    }
>
> private:
>  template <typename TReturn, typename TInput>
>  static inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
> };
>
> const double Math::pi = 3.14;
>
>
>
> 3) Mixed namespace-class approach
> -----------------------------------------------------
> Advantage: Helper functions not accessible to the users, constants are
> defined next to their declaration
> Drawback: More complex code due to template friend functions
>
> namespace Math
>  {
>  const double p = 3.14;
>
>   // Forward declarations
>  class MathDetail;
>  template<typename TReturn, typename TInput>
>  TReturn  Round(TInput, TReturn *);
>
>  class MathDetail
>  {
>  public:
>    template<typename TReturn, typename TInput>
>    friend inline TReturn Round(TInput x, TReturn * = 0);
>
>  private:
>    template <typename TReturn, typename TInput>
>    static inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
>  }
>
>  template <typename TReturn, typename TInput>
>  inline TReturn Round(TInput x, TReturn * = 0)
>    {
>    return MathDetail::RoundHelper<TReturn>(x);
>    }
>  }
>
>
> It seems to me we should wait until after the release to implement any
> of these option. What do you think?
>
> Cheers,
> Tom
>
>
> On Sat, Aug 8, 2009 at 16:18, Bradley Lowekamp<blowekamp at mail.nih.gov>
> wrote:
> > Currently we have two things we would like to put into itk::Math, weather
> it
> > be a namespace or a class. The first is the rounding methods and the
> second
> > is the numeric constants. For the numeric constants consider these two
> > different possibilities:
> >
> >
> > math.h:
> > class Math
> > {
> >        static const double p;
> > }
> >
> > math.cxx:
> > const double p = 3.14;
> >
> > Where as with a namespace, we don't need the static qualifier and I
> believe
> > the following will not have the linking issues, and is valid c++:
> >
> > math.h:
> > namespace Math {
> >        const double p = 3.14;
> > };
> >
> >
> > With the namespace implementation we can always get optimizations, but
> there
> > will be multiple addresses in different compilation files. I don't
> believe
> > that that latter is really a negative though.
> >
> >
> > Brad
> >
> >
> > On Aug 6, 2009, at 11:21 AM, Tom Vercauteren wrote:
> >
> >> Hi Brad,
> >>
> >> Moving these mathematical constants to a more visible place sounds
> >> good to me. However, handling floating point constants in c++ is a
> >> pain. For example, the following code is not standard and won't be
> >> usable on several systems (mac gcc 4.0 at least):
> >>
> >> class No_Good {
> >>  static double const d = 1.0;
> >> };
> >>
> >> I guess that's why boost chose to implement these constants as
> >> template functions:
> >>
> >>
> http://www.boost.org/doc/libs/1_39_0/libs/math/doc/sf_and_dist/html/math_toolkit/toolkit/internals1/constants.html
> >>
> >> The other option is to decouple the definition and declaration as done
> >> in vnl  (with some ugly macro):
> >>
> >> class Now_Better
> >> {
> >>   static double const d;
> >> };
> >>
> >> double const Now_Better::d = 1.0;
> >>
> >> More information can be found here:
> >>
> >>
> http://stackoverflow.com/questions/370283/why-cant-i-have-a-non-integral-static-const-member-in-a-class
> >>
> >> No matter the implementation, I would definitely vote for replacing
> >> the current hard-coded values by a common thing such as vnl_math::pi,
> >> itk::Math::pi, or  itk::Math::pi<double>().
> >>
> >> My two cents,
> >> Tom
> >>
> >> On Thu, Aug 6, 2009 at 16:40, Bradley Lowekamp<blowekamp at mail.nih.gov>
> >> wrote:
> >>>
> >>> Hello,
> >>>
> >>>       I was looking at:
> >>>
> >>> BasicFilters/itkBilateralImageFilter.txx:  rangeGaussianDenom =
> >>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);
> >>>
> >>> It took me a little bit to figure out that we most likely should be
> using
> >>> vnl_math::pi, along with their "e and all that". I do wonder if these
> >>> constants were in itk::Math and in doxygen if it would help with
> >>> increased
> >>> usage.
> >>>
> >>> So I began greping in the "Code" directory and came up with the
> >>> following:
> >>>
> >>> grep -r 3.1415 *
> >>> BasicFilters/itkBilateralImageFilter.txx:  rangeGaussianDenom =
> >>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);
> >>> BasicFilters/itkVectorGradientMagnitudeImageFilter.txx:  const double
> dpi
> >>> =
> >>> 3.14159265358979323846;
> >>> Common/itkGaussianDerivativeSpatialFunction.txx:    prefixDenom *=
> >>> 2*vcl_pow( 2 * 3.1415927, VImageDimension / 2.0);
> >>> Common/itkMersenneTwisterRandomVariateGenerator.h:  double phi = 2.0 *
> >>> 3.14159265358979323846264338328
> >>> Numerics/itkCumulativeGaussianOptimizer.cxx:  m_ComputedAmplitude =
>  sum
> >>> /
> >>> (m_ComputedStandardDeviation * vcl_sqrt(2*3.14159265));
> >>>
> >>> grep -r 0.707 *
> >>> BasicFilters/itkBSplineResampleImageFilterBase.txx:      m_G[0]  =
> >>>  0.707107;
> >>>
> >>> grep -r 1.414 *
> >>> Numerics/Statistics/itkGaussianDistribution.cxx:    dq  = 0.5e+0 *
> >>> vnl_erfc(
> >>> dx / 1.414213562373095e+0 ) - dp;
> >>> Review/Statistics/itkGaussianDistribution.cxx:    dq  = 0.5e+0 *
> >>> vnl_erfc(
> >>> dx / 1.414213562373095e+0 ) - dp;
> >>>
> >>> grep -r vnl_math:: * | wc
> >>>     69     571    7652
> >>>
> >>> Well I guess that is almost 90% usage of the numeric constants, so its
> >>> not
> >>> bad at all.
> >>>
> >>> I'll work on a patch tonight, and likely commit something tomorrow if
> no
> >>> one
> >>> sees a problem with this.
> >>>
> >>> Brad
> >>> _______________________________________________
> >>> Powered by www.kitware.com
> >>>
> >>> Visit other Kitware open-source projects at
> >>> http://www.kitware.com/opensource/opensource.html
> >>>
> >>> 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
> >>>
> >
> >
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090811/22a439ec/attachment.htm>


More information about the Insight-developers mailing list