[Insight-developers] Class vs Namespace
Tom Vercauteren
tom.vercauteren at m4x.org
Tue Aug 11 10:13:10 EDT 2009
Hi Luis,
The pointer is actually a dummy argument, it is not necessary nor
used. It is only meant to work around the msvc 6 bug you were
mentioning a few days ago. This is why this second argument has a
default value but no name. For clarity, I should maybe have added a
"itkNotUsed" in there. Maybe something like
inline TReturn RoundHelper(TInput x, TReturn *
itkNotUsed(dummyPtrForMSVC6Bug) = 0)
instead of
inline TReturn RoundHelper(TInput x, TReturn * = 0)
The goal is really to get something as simple as
int k = Math::Round<int>( somedouble );
Sorry if this wasn't clear.
Tom
On Tue, Aug 11, 2009 at 15:52, Luis Ibanez<luis.ibanez at kitware.com> wrote:
>
> 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
>
>
More information about the Insight-developers
mailing list