views:

178

answers:

2

Hi
in this question:
http://stackoverflow.com/questions/2779155/template-point2-double-point3-double
Dennis and Michael noticed the unreasonable foolishly implemented constructor.
They were right, I didn't consider this at that moment. But I found out that a constructor does not help very much for a template class like this one, instead a function is here much more convenient and safe

namespace point {

template < unsigned int dims, typename T >
struct Point {

    T X[ dims ];

    std::string str() {
        std::stringstream s;
        s << "{";
        for ( int i = 0; i < dims; ++i ) {
            s << " X" << i << ": " << X[ i ] << (( i < dims -1 )? " |": " ");
        }
        s  << "}";
        return s.str();
    }

    Point<dims, int> toint() {
        Point<dims, int> ret;
        std::copy( X, X+dims, ret.X );
        return ret;
    }
};

template < typename T >
Point< 2, T > Create( T X0, T X1 ) {
    Point< 2, T > ret;
    ret.X[ 0 ] = X0; ret.X[ 1 ] = X1;
    return ret;
}
template < typename T >
Point< 3, T > Create( T X0, T X1, T X2 ) {
    Point< 3, T > ret;
    ret.X[ 0 ] = X0; ret.X[ 1 ] = X1; ret.X[ 2 ] = X2;
    return ret;
}
template < typename T >
Point< 4, T > Create( T X0, T X1, T X2, T X3 ) {
    Point< 4, T > ret;
    ret.X[ 0 ] = X0; ret.X[ 1 ] = X1; ret.X[ 2 ] = X2; ret.X[ 3 ] = X3;
    return ret;
}
};
int main( void ) {
    using namespace point;
    Point< 2, double > p2d = point::Create( 12.3, 34.5 );
    Point< 3, double > p3d = point::Create( 12.3, 34.5, 56.7 );
    Point< 4, double > p4d = point::Create( 12.3, 34.5, 56.7, 78.9 );
    //Point< 3, double > p1d = point::Create( 12.3, 34.5 ); //no suitable user defined conversion exists

    //Point< 3, int > p1i = p4d.toint(); //no suitable user defined conversion exists
    Point< 2, int > p2i = p2d.toint();
    Point< 3, int > p3i = p3d.toint();
    Point< 4, int > p4i = p4d.toint();

    std::cout << p2d.str() << std::endl;
    std::cout << p3d.str() << std::endl;
    std::cout << p4d.str() << std::endl;
    std::cout << p2i.str() << std::endl;
    std::cout << p3i.str() << std::endl;
    std::cout << p4i.str() << std::endl;

    char c;
    std::cin >> c;
}  

has the new C++ standard any new improvements, language features or simplifications regarding this aspect of ctor of a template class?
what do you think about the implementation of the combination of namespace, stuct and Create function?
many thanks in advance
Oops

+1  A: 

Yes, as Michael pointed out in his answer to your previous question, in C++0x you'll be able to use an initializer list to pass an arbitrary number of arguments to your ctor. In your case, the code would look something like:

template <int dims, class T>
class point { 
    T X[dims];
public:
    point(std::initializer_list<T> const &init) { 
        std::copy(init.begin(), init.begin()+dims, X);
    }
};

You could create a point object with this something like:

point<3, double> x{0.0, 0.0, 0.0};

Personally, I'm not sure I like the basic design very well though. In particular, I'd rather see X turned into an std::vector, and determine the number of dimensions strictly from the parameter list that was passed instead of having it as a template argument:

template <class T>
class point { 
    std::vector<T> X;
public:
    point(std::initializer_list<T> init) {
        std::copy(init.begin(), init.end(), std::back_inserter(X));
    }
};

This does have some trade-offs though -- points with a different number of dimensions are still the same type. For example, it basically asserts that it's reasonable to assign a 2D point to a 3D point, or vice versa.

Jerry Coffin
Shouldn't you check that the `initializer_list` has a suitable number of items? Also, why not use a `std::array`? (Too bad `initializer_list` implementation is completely broken with my version of GCC.)
UncleBens
@UncleBens:Yes, you should check that the initializer_list is large enough. `std::array` would probably make sense here too -- I'm just not accustomed to using it yet...
Jerry Coffin
@Jerry: Also I suggested variadic templates. But can you check the size of `initializer_list` at compile-time? - I also think `vector` might not be very suitable here: one wants these things to be light-weight, and the dynamic allocation and space overhead might be too much ... to store 2 or 3 doubles/ints.
UncleBens
Yep the first and second (vector) can both be written as `template<typename ...T> point(T const` whereas the variadic template won't and will require `x{0, 0, 0}` or `x(0, 0, 0)`.
Johannes Schaub - litb
+3  A: 

Since the array is public, it is an option to omit the constructor and allow aggregate initialization (like boost::array<T, N> for example).

Point<2, int> p = {1, 2};

This is no worse than having to call a create function. (The create function might still be handy as a utility.)


In C++0x you will be able to have all sorts of coolness. For example, play with variadic templates, to have it checked at compile-time if the constructor is called with a right number of arguments. (The following could also check if the arguments ...U are all of type T, with some more metaprogramming fun, but it might not be absolutely necessary.)

//helper to copy variable amount of arguments into an array
namespace detail {

template <class T, class U>
void copy_variadic(T* p, U value)
{
    *p = value;
}

template <class T, class First, class ...Rest>
void copy_variadic(T* p, First var, Rest ...args)
{
    *p = var;
    copy_variadic(++p, args...);
}
} //detail

template < unsigned int dims, typename T >
struct Point {

    T X[ dims ];

    Point() : X{}
    {
    }

    template <class ...U>
    Point(U... args) {
        static_assert(sizeof...(args) == dims, "Too many or too few arguments to Point constructor");
        detail::copy_variadic(X, args...);
    }
    //...
};

(Actually, with some modifications - perfect forwarding - copy_variadic would make a nice addition to my collection of variadic-template utilities, if someone doesn't come and point out a significantly better way.)

UncleBens
You can directly initialize arrays with C++0x and GCC4.5: `template<class ...U> Point(U... args):X{move(args)...} { }`. That will also automatically check that not too many initializers are provided :)
Johannes Schaub - litb
@Johannes: Thanks, never occurred to me. - However, isn't std::move wrong there? `template<class ...U> Point(U }`. But it'd still be useful as a utility, because it allows copying to any output iterator?
UncleBens
@UncleBens having already copied the arguments into the parameters, one can safely move them though, but i believe there is little reason not to take the forwarding way. Both ways will end up doing essentially the same, i think. Just the time when they do the copy/move is different. If you know that all elements have non-throw move, then the copy-early way is exception safe, but that doesn't really matter for constructors :) I think the utility you wrote could be useful indeed :)
Johannes Schaub - litb
@Johannes: Theoretically speaking (because in case of a Point it all doesn't matter at all), doesn't forwarding mean that no copying will occur at all, if you call it with rvalues?
UncleBens
@Uncle indeed. But in neither version a copy will be done in that case. The rvalue is moved into the parameter, and then moved into the array. Or if the compiler elides the first move, there is a single move into the array. As is with the forwarding case. I can see that for constructors (exception safety doesn't matter), the forwarding way has the benefit that it does always only one move, no matter whether the compiler optimizes or not. But i heard it's a pretty common optimization to construct temporaries directly into by-value parameters.
Johannes Schaub - litb
Or wait, i think i see now what you mean. You mean that if the array is of element type `T`, and if i pass arguments to the constructor that can be converted to `T`. I see now that indeed the forwarding way should be used. I missed that all the way :)
Johannes Schaub - litb
@Johannes: I wasn't actually thinking much at all. If you invoke it with temporaries (`Point<Foo> x(Foo(), Foo());`), isn't the sequence in one case *default -> **copy** -> move* and in the second case *default -> move*? That is, assuming worst case of no constructor eliding from the compiler. (Also, it is possible to have types that are not copyable, but can be moved.)
UncleBens
@Uncle the arguments are rvalues and will be moved into the parameters :) So in the one case it is *default* -> *move* -> *move*. The non-forward way has the downside tho that "copyable-only" elements will be copied twice, while with forwarding only one copy will take place.
Johannes Schaub - litb
@Johannes: Ok, then. As it is, I'm only getting "bad array initializer" with moveable-only types (GCC 4.4), so either the compiler is wrong or arrays can only store copyable types. But when initializing a simple member, it appears that the copy constructor is indeed not required.
UncleBens