Help with my vector class

[u8sand]

I programmed this vector class to support different types and different amount of dimensions. (class T,int D) D is the dimension.

I don't use templates much and I'm running into an error, my constructors aren't working.

Here is my class

#include <stdio.h>
#include <stdarg.h>

template <class T,int D>
class Vector
{
public:
    Vector();
    Vector(...);
    Vector(const Vector<T,D>& vec);
    ~Vector();

    const Vector<T,D>& operator=(Vector<T,D>);

    Vector<T,D> operator+(Vector<T,D>);
    Vector<T,D> operator-(Vector<T,D>);
    Vector<T,D> operator*(Vector<T,D>);
    Vector<T,D> operator/(Vector<T,D>);

    const Vector<T,D>& operator+=(Vector<T,D>);
    const Vector<T,D>& operator-=(Vector<T,D>);
    const Vector<T,D>& operator*=(Vector<T,D>);
    const Vector<T,D>& operator/=(Vector<T,D>);

    bool operator==(Vector<T,D>);
    //bool operator==(T);

    //bool operator> (Vector<T,D>);
    //bool operator< (Vector<T,D>);
    //bool operator>=(Vector<T,D>);
    //bool operator<=(Vector<T,D>);
    //bool operator> (T);
    //bool operator< (T);
    //bool operator>=(T);
    //bool operator<=(T);

    T& operator[](const int& index);

    //operator const T();
private:
    //int Length();

    T* dim;
};

template <class T,int D>
Vector<T,D>::Vector()
{
    if(D > 0)
    {
        dim = new T[D];
        for(int i = 0; i < D; i++)
            dim[i] = 0;
    }
}
template <class T,int D>
Vector<T,D>::Vector(...)
{
    if(D > 0)
    {
        va_list dims;
        va_start(dims,D);
        dim = new T[D];
        for(int i = 0; i < D; i++)
            dim[i] = va_arg(dims,T);
        va_end(dims);
    }
}
template <class T,int D>
Vector<T,D>::Vector(const Vector<T,D>& vec)
{
    if(D > 0)
    {
        dim = new T[D];
        for(int i = 0; i < D; i++)
            dim[i] = vec.dim[i];
    }
}
template <class T,int D>
Vector<T,D>::~Vector()
{
    if(D > 0)
        delete [] dim;
}
template <class T,int D>
const Vector<T,D>& Vector<T,D>::operator=(Vector<T,D> v)
{
    if(D > 0)
    {
        delete [] dim;
        dim = new T[D];
        for(int i = 0; i < D; i++)
            dim[i] = v.dim[i];
    }
    return *this;
}
template <class T,int D>
Vector<T,D> Vector<T,D>::operator+(Vector<T,D> v)
{
    Vector<T,D> _v;
    for(int i = 0; i < D; i++)
        _v.dim[i] = dim[i] + v.dim[i];
    return _v;
}
template <class T,int D>
Vector<T,D> Vector<T,D>::operator-(Vector<T,D> v)
{
    Vector<T,D> _v;
    for(int i = 0; i < D; i++)
        _v.dim[i] = dim[i] - v.dim[i];
    return _v;
}
template <class T,int D>
Vector<T,D> Vector<T,D>::operator*(Vector<T,D> v)
{
    Vector<T,D> _v;
    for(int i = 0; i < D; i++)
        _v.dim[i] = dim[i] * v.dim[i];
    return _v;
}
template <class T,int D>
Vector<T,D> Vector<T,D>::operator/(Vector<T,D> v)
{
    Vector<T,D> _v;
    for(int i = 0; i < D; i++)
        _v.dim[i] = dim[i] / v.dim[i];
    return _v;
}
template <class T,int D>
const Vector<T,D>& Vector<T,D>::operator+=(Vector<T,D> v)
{
    for(int i = 0; i < D; i++)
        dim[i] += v.dim[i];
    return *this;
}
template <class T,int D>
const Vector<T,D>& Vector<T,D>::operator-=(Vector<T,D> v)
{
    for(int i = 0; i < D; i++)
        dim[i] -= v.dim[i];
    return *this;
}
template <class T,int D>
const Vector<T,D>& Vector<T,D>::operator*=(Vector<T,D> v)
{
    for(int i = 0; i < D; i++)
        dim[i] *= v.dim[i];
    return *this;
}
template <class T,int D>
const Vector<T,D>& Vector<T,D>::operator/=(Vector<T,D> v)
{
    for(int i = 0; i < D; i++)
        dim[i] /= v.dim[i];
    return *this;
}
template <class T,int D>
bool Vector<T,D>::operator==(Vector<T,D> v)
{
    for(int i = 0; i < D; i++)
        if(dim[i] != v.dim[i])
            return false;
    return true;
}

template <class T,int D>
T& Vector<T,D>::operator[](const int& index)
{
    if(index >= 0 && index < D)
        return dim[index];
    return (T*)0;
}



typedef Vector<int,2> Vector2D;
typedef Vector<int,3> Vector3D;
typedef Vector<int,4> Vector4D;

And a basic initialization:

Vector2D v(1,2);

here are the errors i'm receiving:

\main.cpp||In function 'int main(int, char**)':|
\main.cpp|8|error: call of overloaded 'Vector()' is ambiguous|
\vector.h|59|note: candidates are: Vector<T, D>::Vector(...) [with T = int, int D = 2]|
\vector.h|49|note:                 Vector<T, D>::Vector() [with T = int, int D = 2]|
\main.cpp|8|warning: unused variable 'v'|
||=== Build finished: 3 errors, 1 warnings ===|

Thank you in advanced. Also if you see anything in my code that needs to be fixed feel free to tell me about them.

[nmaillet]

You cannot have a Vector() and a Vector(...) constructor. Constructors with a variable number of parameters could have 0 parameters. Which is equivalent to Vector() . The compiler is uncertain of which constructor to call. You have to remove the parameter-less constructor and perform a check in the other constructor for the number of parameters passed. Hope that helps.

[u8sand]

Thank you so much, such a simple error i overlooked. Everything works now.

[mike_2000_17]

nmaillet is correct.

But in response to:
>>Also if you see anything in my code that needs to be fixed feel free to tell me about them.

I don't see anything that _needs_ to be fixed, but certain things could be made better.

First, think hard before you use the var-args feature. This is a C feature that is not type-safe. Nothing prevents someone from constructing a Vector and give it a bunch of crazy parameters that will corrupt everything. Type-safety is a blessing for robustness, by-passing type-safety is not something to be done lightly. I think I understand what you are trying to do with this constructor: be able to construct a vector by giving it a list of values. Personally, I would go with the good old fashion way:

Vector(T q1) {
     BOOST_STATIC_ASSERT(D == 1);
     //..
   };
   Vector(T q1, T q2) {
     BOOST_STATIC_ASSERT(D == 2);
     //..
   };
   Vector(T q1, T q2, T q3) {
     BOOST_STATIC_ASSERT(D == 3);
     //..
   };
   //.. etc. for as many as you like.

Of course, if you can use C++0x (upcoming standard), then you can simply use the initializer_list. As so:

Vector(std::initializer_list<T> q);

Second, if you know the length of the vector from a template argument (D), then why don't you make your vector class store a static array? Why pay the price of dynamic memory allocation when it is not necessary? I highly recommend you do that, to actually make that class useful in any way.

Third, all your operator overloads should take const-references. Why do you need to copy read-only parameters? Take them by reference.

Fourth, all overloaded operators which are not required to be member functions should not be. This can be quite important for argument-dependent look-up of the overloaded operators. Make them friend functions instead (or just nonmember functions).

Fifth, you need to provide const versions for your member functions, especially the indexing operator:

T& operator[](int index); //provide non-const version
  const T& operator[](int index) const; //AND const version.

By the way, in the above, I removed the const-ref passing of the index, because it is just silly to pass an int by reference (creating, passing and dereferencing a const-ref parameter is more expensive than just copying an int).

If you choose to implement comparison operators as member functions, they also should be marked as const. In general, use const-ness proactively, that's the best advice. By the way, how do you determine if a vector is less than another? (as you commented-out some comparison operators) I'm curious.

Sixth, I suggest you try to achieve some inter-operability with STL containers as well. I would suggest you implement functions similar (with same names) as those of the STL container (like std::vector), and implement an iterator (just a pointer) with begin() and end() functions. This will increase the usefulness of your class. Also, I suggest you allow construction of your objects in a STL-like fashion, that is, I would suggest you add this overloaded constructor:

template <typename ForwardIter>
    Vector(ForwardIter first, ForwardIter last) {
      //..
    };

Such that you can initialize a Vector with elements contained in an STL container or similar (anything with an iterator concept).

Seventh, whenever writing class templates, it is good practice to define all the relevant types as nested typedefs, as so:

template <class T,int D>
class Vector {
  public:
    typedef Vector<T,D> self;
    typedef T value_type;
    typedef std::size_t size_type;
    
    typedef T* pointer;
    typedef const T* const_pointer;
    typedef T& reference;
    typedef const T& const_reference;
    typedef T* iterator;
    typedef const T* const_iterator;

This will allow you not only to be compatible with STL code, but it is easier to change types later. Basically, with the above, you want to use only those nested typedefs in the entire implementation AND interface of your class template. This habit is very useful, believe me, it might not be obvious why it is not "just a waste of time" to put up this list of nested types, but you will realize soon enough.

Finally, again for interoperability, you might want to consider relying on overloaded operator templates and the use of type-traits (and Sfinae switches). If what I just wrote went way over your head, then forget it and don't worry about it, for now.