Ticket #1240 (closed enhancement: fixed)

Opened 10 years ago

Last modified 9 years ago

Request for direction construction of Points and Extents

Reported by: rowen Owned by: jbosch
Priority: normal Milestone:
Component: afw Keywords:
Cc: rhl, rowen, bick, fergal Blocked By:
Blocking: Project: LSST
Version Number: 3.3.22
How to repeat:

not applicable

Description

Please support direct construction of points and extents from scalar value:

Point2I(int, int) Extent2I(int, int)

If this is doable and practical then please also deprecate ::make and the factory functions.

The benefit for users is:

  • Simpler API: the direct technique works so there's no need to remember "::make" or the various factory functions.
  • It matches what we are used to from the old Point
  • I suspect it matches user expectations (though I can only speak for mine)
  • fewer names in the namespace

Change History

comment:1 Changed 10 years ago by DefaultCC Plugin

  • Cc rhl, rowen, bick, fergal added

comment:2 Changed 10 years ago by jbosch

I'm annoyed by this too, and I agree with all your reasons, but I couldn't find a good way to accomplish it. Here are the alternatives I see:

  • Option 1: Make Point2X, Point3X, etc. entirely separate classes, not typedefs of a template Point<T,N> class (or maybe a template class with only explicit specializations). This becomes more palatable if we use some macro language to generate the code, but otherwise it's a lot of hard-to-maintain repetition.
  • Option 2: Give Point<T,N> a constructor with default arguments:
       Point<T,N>::Point(T x, T y=0.0, T z=0.0);
    

That's not horrible, although it does allow the nonsensical Point2D(1.0, 2.0, 3.0).

I think this will get solved in C++0x, so I'm not strongly opposed to this solution as a temporary, if other people aren't bothered by it.

comment:3 Changed 10 years ago by rowen

I see your point.

I think your solution of a nonsensical "z" argument is tolerable if PointI and PointD are removed entirely (which I support anyway!). Otherwise you have the horrible situation of PointI(1.0, 2.0, 3.0) generating a 2d point instead of the 3d point that anybody would expect. At least Point2I(1.0, 2.0, 3.0) looks funky.

Another possibility is two constructors, as follows:

  • Point<T,N>::Point(T x, T y)
  • Point<T,N>::Point(T x, T y, T z)

The first one refuses to compile if N != 2 or at least raises an exception when called (if either can be done with template programming). Ditto for the latter if N != 3.

But yes, it would be nice if there was an efficient way to make them separate classes.

comment:4 Changed 10 years ago by jbosch

Can you provide an example of how to make a non-template constructor in a template class refuse to compile based on its arguments? The only technique I can come up with requires the constructor to be templated:

template <typename T, int N>
class Point {
public:

    template <typename U>
    Point(U x, U y) {
        BOOST_STATIC_ASSERT(N==2);
    }

    template <typename U>
    Point(U x, U y, U z) {
        BOOST_STATIC_ASSERT(N==3);
    }

};

It might be better to use SFINAE failure (boost::enable_if) instead of static assert, but either way it looks like the extra template parameter is necessary - otherwise the static assert or SFINAE failure gets triggered when the whole class is explicitly instantiated.

I also don't look forward to trying to make SWIG understand this. I'm beginning to think the best solution is just to explicitly specialize Point for different dimensions, remove the CRTP base class, and accept the code duplication.

comment:5 Changed 9 years ago by jbosch

  • Status changed from new to closed
  • Resolution set to fixed
  • reviewstatus changed from notReady to selfReviewed

Fixed in branch for #1556.

Note: See TracTickets for help on using tickets.