Header file optimization

Heya, I'm just creating some random stuff to kill time and I was just wondering if there was anyway to further optimize or in anyway improve the following code: (don't be discouraged by it's length, it's really easy to understand)

/* 
 * File:   CVector.h
 * Author: T. Westendorp
 *
 * Description:
 * Defines functionality of mathemathical vector calculations.
 *
 * Important note:
 * Setting the length of a null vector can lead to unexpected behavior.
 * (doubles holding values of: -nan)
 *
 * Automatically includes: <cmath> (if not earlier included)
 */

#ifndef cmath
#include <cmath>
#endif

#ifndef _CVECTOR_H

#define PI 3.141592653
class CVector
{
private:
    double * x; // Dynamically allocated double as x coordinate
    double * y; // Dynamically allocated double as y coordinate
public:
    CVector(); // Standard constructor
    CVector(const double&,const double&); // Overloaded constructor
    CVector(const CVector&); // Copy constructor
    ~CVector(); // Standard destructor
    CVector operator + (const CVector&) const; // Add vectors
    CVector operator - (const CVector&) const; // Substract vectors
    CVector operator / (const double&) const; // Divide by a scalar
    CVector operator * (const double&) const; // Multiply by a scalar
    CVector& operator = (const CVector&); // Vector equation
    CVector& operator += (const CVector&); // Vector addition
    CVector& operator -= (const CVector&); // Vector subtraction
    CVector& operator /= (const double&); // Scalar division
    CVector& operator *= (const double&); // Scalar multiplication
    operator double() const; // Convert to double
    operator int() const; // Convert to integer
    void SetX(const double&); // Set x endcoordinate
    void SetY(const double&); // Set y endcoordinate
    void SetLen(const double&); // Set length (using the current angle)
    double GetX(void) const; // Get x endcoordinate
    double GetY(void) const; // Get y endcoordinate
    double GetDir(void) const; // Get the offset 'theta' (aka angle)
    double GetLen(void) const; // Get the length of the vector
};

CVector::CVector()
{
    x = new double(0.0);
    y = new double(0.0);
}

CVector::CVector(const double& xx, const double& yy = 0.0)
{
    x = new double(xx);
    y = new double(yy);
}

CVector::CVector(const CVector& v)
{
    x = new double(*(v.x));
    y = new double(*(v.y));
}

CVector::~CVector()
{
    delete x;
    delete y;
}

CVector CVector::operator + (const CVector& v) const
{
    return CVector ( (*x) + v.GetX() , (*y) + v.GetY() );
}

CVector CVector::operator - (const CVector& v) const
{
    return CVector ( (*x) - v.GetX() , (*y) - v.GetY() );
}

CVector CVector::operator / (const double& d) const
{
    return CVector ( (*x) / d , (*y) / d );
}

CVector CVector::operator * (const double& d) const
{
    return CVector ( (*x) * d , (*y) * d );
}

CVector& CVector::operator = (const CVector& v)
{
    if (this != &v)
    {
        *x = *(v.x);
        *y = *(v.y);
    }
    return *this;
}

CVector& CVector::operator += (const CVector& v)
{
    *x += *(v.x);
    *y += *(v.y);
    return *this;
}

CVector& CVector::operator -= (const CVector& v)
{
    *x -= *(v.x);
    *y -= *(v.y);
    return *this;
}

CVector& CVector::operator /= (const double& d)
{
    *x /= d;
    *y /= d;
    return *this;
}

CVector& CVector::operator *= (const double& d)
{
    *x *= d;
    *y *= d;
    return *this;
}

CVector::operator double() const
{
    return GetLen();
}

CVector::operator int() const
{
    return GetLen();
}

void CVector::SetX(const double& xx)
{
    *x = xx;
}

void CVector::SetY(const double& yy)
{
    *y = yy;
}

void CVector::SetLen(const double& d)
{
    (*this)/=GetLen();
    (*this)*=d;
}

double CVector::GetX(void) const
{
    return (*x);
}

double CVector::GetY(void) const
{
    return (*y);
}]

double CVector::GetDir(void) const
{
    return (atan((*y)/(*x)) / PI * 180);
}

double CVector::GetLen(void) const
{
    return (sqrt((*x)*(*x)+(*y)*(*y)));
}
#endif

I'm also open to suggestion on what to add or even, on what subject to focus my next math-oriented header file.

filipe (1165)

As far as I remember, this is not necessary:

1
2
3

#ifndef cmath
#include <cmath>
#endif

I believe standard headers have their own include guards.

The other include guard is missing the #define _CVECTOR_H line. As it is, you never define _CVECTOR_H. Also, identifiers beginning with the '_' character are reserved by the standard.

Also notice that you're using a macro hack to define pi, when a const double would be a much better (and type safe) option.

Athar (4466)

A major problem is that you're dynamically allocating x and y, even though there is no apparent reason to do so.

moorecm (1932)

You might want to see about using an initialization list in your constructor(s).

Also, some operators are better left outside of the class as friends. If I am remembering correctly, operator+, operator-, operator*, and operator/ are some of them. It has to do with associativity...

Galik (2254)

The top of your file is not very efficient. This should be better:

#ifndef _CVECTOR_H
#define _CVECTOR_H // remember to make this define

// place your includes inside your include guards
#include <cmath> // do not conditionally include them

class CVector
{

Galik (2254)

Why the dynamic allocation?

class CVector
{
private:
    double * x; // Dynamically allocated double as x coordinate
    double * y; // Dynamically allocated double as y coordinate

This should be safer and quicker

class CVector
{
private:
    double x; // no dynamic allocation
    double y;

jsmith (5804)

unary minus?

use boost::operators where possible

provide friends so you can do things like 2 * v (I believe you support only v * 2).

providing both operator int() and operator double() is evil. (provide neither;
prefer an explicit accessor).

Topic archived. No new replies allowed.

C++

Forum

Header file optimization