[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