Small 3D and 4D vector utilities

Question

I'm a hobby programmer and have started 2 month ago with C++. I had some knowledge of Python, and I learned it all through Internet tutorials.

I'm looking for your help, since I don't have a tutor I would like to have a comment about my programming style. I used this small "library" to create the Snake4D game (see bottom of question for further information).

Here is a little program that handles mathematical vectors, 4D and 3D. Specifically, I ask you to find any improvements in the code's readability and in the algorithms.

Here is the "vec.h" file:

class V4{
private:
    double coords[4];
 public:
    V4();
    V4(double x, double y, double z, double w);
    ~V4();

    void set_cooord(int i, double x); //set_coord (index of coord i, value x)

    double module();
    double operator[](unsigned int i);

    void operator =(V4);
    V4 operator +(const V4);
    V4 operator -(const V4);
    V4 operator *(double k); //moltiplication of vector by a scalar k

    double dot4(V4); //the dot product

    V4 cross(V4 u, V4 v,V4 w); //the cross product
};

Here is the "vec.cpp" file:

/****************
V4 class
*****************/
V4::V4(){
    //set all coords to 0. if it's called without specific coord
    coords[0] = 0.;
    coords[1] = 0.;
    coords[2] = 0.;
    coords[3] = 0.;
}

V4::V4(double x, double y, double z, double w){
    coords[0] = x;
    coords[1] = y;
    coords[2] = z;
    coords[3] = w;
}

V4::~V4(){}//nothing to destroy?

void V4::set_cooord(int i, double x){ //i index, x value
    coords[i] = x;
}

double V4::module(){
    double sum = 0;
    for(int i= 0; i<4; i++){
        sum += coords[i]*coords[i];
    }
    return sqrt(sum);
}

double V4::operator[](unsigned int i){
     //if(i > 4){return;}  //list index out of range ? how to throw give a compile error?
     return coords[i];
 }

void V4::operator =(V4 v1){
    for(int i= 0; i<4; i++){
        coords[i] = v1[i];
    }
}

V4 V4::operator+(V4 v1) {
    double c[4]; // array of 4 double which will be transfered to the returned vector
     for(int i= 0; i<4; i++){
         c[i] = coords[i] + v1[i];
     }
     return V4(c[0],c[1],c[2],c[3]);
 }

 V4 V4::operator-( V4 v1) {
     double c[4];
     for(int i= 0; i<4; i++){
         c[i] = coords[i] - v1[i];
     }
     return V4(c[0],c[1],c[2],c[3]);
 }


V4 V4::operator*(double k){
    double c[4];
    for(int i= 0; i<4; i++){
        c[i] = coords[i] * k;
    }
    return V4(c[0],c[1],c[2],c[3]);
}

double V4::dot4(V4 v1){
    double sum = 0;
    for(int i= 0; i<4; i++){
       sum += coords[i]*v1[i];
    }
    return sum;
}


// requires three external vector and doesn't take any coords of the actual vector
//intended use:
// V4 v1(x,y,z,w), v2(x,y,z,w), v3(x,y,z,w), result, op;
// result = op.cross(v1,v2,v3)
V4 V4::cross(V4 u, V4 v, V4 w){
    //source Steve Hollasch master thesis
    //partial product
    double A = (v[0]*w[1])-(v[1]*w[0]);
    double B = (v[0]*w[2])-(v[2]*w[0]);
    double C = (v[0]*w[3])-(v[3]*w[0]);
    double D = (v[1]*w[1])-(v[2]*w[1]);
    double E = (v[1]*w[3])-(v[3]*w[1]);
    double F = (v[2]*w[3])-(v[3]*w[2]);

    double x =  (u[1]*F)-(u[2]*E)+(u[3]*D);
    double y = -(u[0]*F)+(u[2]*C)-(u[3]*B);
    double z =  (u[0]*E)-(u[1]*C)+(u[3]*A);
    double wc = -(u[0]*D)+(u[1]*B)-(u[2]*A); //defined as wc to not conflic with w vector

    return V4(x,y,z,wc);
}

Here are the source (.h and .cpp) file.

I already see some improvements:

1. I use the vectors as public members, because if they are private, I cannot get access to all the really comfortable functions they offer.
2. Instead of giving vertex hand by hand for the generation of the hypercube, there should be an algorithm that does this.
3. It could be used a general purpose class V containing the basic methods and having a variable int dimension then specialize the V3 and V4 inheriting the class V.

PS: I've made a little clip that shows the Snake4D game written in C++, Qt and OpenGL.

Answer 1

Very good.

Couple of points:

I would remove the set() method, and allow its work to be done by the operator[]. To do this you need return the value you want by reference.

double &  operator[](unsigned int i);
  //  ^^^  Return double by reference.

This allows you to do;

V4 x;
x[1] = 12;

When you pass objects as parameters it is usally best to pass by const reference. This means that no copy will be made and the original object can not be modified by the method using it:

V4 operator +(V4 const & rhs);
                      ^^^

Note I always put the const on the right of the type (rather than the left). Admittedly this is a religious war so choose a side and stick to it (there is only one minor side instance were it makes difference). But I find it makes reading the type name easier (as types are read from right to left with const always binding to the left (except where it is the leftmost lexeme in the type then it binds right).

Methods that do not change the state of the object should be declared const:

V4 operator -(V4 const & rhs) const;
                        //    ^^^^^  indicates that calling this method will not
                        //           change the state of the object.

Note: This means you should provide two versions of operator[] a normal one that allows mutation and a const version that can be used in contexts where it is read only.

double&        operator[](unsigned int i)        { return coord[i];}
double const&  operator[](unsigned int i) const  { return coord[i];}

Note: array access is not validated against the array bounds. Normally you don't check as we do not want to impose an extra burden on the responsible just because there are bad developers. But it is traditional to add at() method that does the same work as operator[] but also validates the bounds of the array access.

double&        at(unsigned int i)        { if (i > 3) {throw std::range_error("BLA");} return coord[i];}
double const&  at(unsigned int i) const  { if (i > 3) {throw std::range_error("BLA");} return coord[i];}

Implementing operator + - * / is easily done if you implement them in terms of += -= *= /=. This also provides the user of your object the opportunity to use your class more efficiently when a new object is not required (though this should be a secondary though).

V4 operator *(V4 const & rhs) const;
V4 operator *=(V4 const & rhs);       // This does change the sate of the object

V4 V4::operator*(V4 const& rhs) const { V4 result(*this); result *= rhs; return result;}

Using a for loop is easy. But sometimes it can be simpler to use the standard algorithms to accomplish simple tasks over short containers:

double sum = 0;
for(int i= 0; i<4; i++){
    sum += coords[i]*coords[i];
}

This can be replaced with:

sum = std::accumulate(coords, coords + 4, 0);

You can find good documentation here: http://www.sgi.com/tech/stl/table_of_contents.html (look for section: 5 Algorithms)

In the moto of not doing extra work. Don't write methods that do nothing:

V4::~V4(){}//nothing to destroy?

Remove this. The class automatically provides a desttructor that will correctly destroy nothing.

I actually like using void as the return type of assignment (I think it has no down side). But traditionally assignment should return a reference to self. This then allows the object to be part of an assignment chain.

V4  a;
V4  b;
V4  c;

a = b = c; // assignment chaining.
// It also allows you to pass the object as a parameter to a mehtod:

doStuff( a = b); // Assign a to b. Then call doStuff using a.

It is also a good idea to define the streaming operators for most classes. This allowsd you to serialize and de-serialize the class to a stream (file/string etc).

So now the class looks like this.

class V4
{
    double coords[4];
 public:
    V4();
    V4(double x, double y, double z, double w);

    double module() const;
    double&       operator[](unsigned int i)       {return coords[i];}
    double const& operator[](unsigned int i) const {return coords[i];}

    V4&  operator = (V4 const& rhs);
    V4&  operator +=(V4 const& rhs);
    V4&  operator -=(V4 const& rhs);
    V4&  operator *=(double k); //moltiplication of vector by a scalar k

    V4   operator +(V4 const& rhs)  const {V4 result(*this); result += rhs; return result;}
    V4   operator -(V4 const& rhs)  const {V4 result(*this); result -= rhs; return result;}
    V4   operator *(double rhs)     const {V4 result(*this); result *= rhs; return result;}

    double dot4(V4 const& rhs) const; //the dot product

    V4 cross(V4 const& u, V4 const& v,V4 const& w) const; //the cross product
};

Then rest of the interesting ones:

double V4::module() const
{
    return std::sqrt(std::accumulate(coords, coords + 4, 0.0);
}

V4&   V4::operator +=(V4 const& rhs)
{
    std::transform(coord, coord+4, rhs.coord, coord, std::plus);
    return *this;
}
V4&   V4::operator -=(V4 const& rhs)
{
    std::transform(coord, coord+4, rhs.coord, coord, std::minus);
    return *this;
}
V4&   V4::operator *=(double k)
{
    std::transform(coord, coord+4, coord, std::bind1st(std::multiplies, k));
    return *this;
}