I'd like a critique of my mathematical vector struct

Armendhammer (11)

I wrote a struct with operator overloading as practice, I want to know what I've done wrong and how I can make it better.
Edit: would this thread be better suited in the beginners section?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
 
struct vec3d
{
	double x, y, z;
	//vec3d() : x(), y(), z() {}
	//vec3d(double x, double y, double z) : x(x), y(y), z(z) {}
	vec3d(double x, double y, double z) { this->x = x; this->y = y; this->z = z; }
	vec3d() {};
	vec3d& operator = (const vec3d &rhs);
	bool operator == (const vec3d &rhs) const;
	bool operator != (const vec3d &rhs) const;
        //from a maths standpoint, I'm not sure how much sense the operators below make
	bool operator > (const vec3d &rhs) const;
	bool operator < (const vec3d &rhs) const;
	bool operator >= (const vec3d &rhs) const;
	bool operator <= (const vec3d &rhs) const;
};

vec3d& vec3d::operator = (const vec3d &v)
{
	if (!(this == &v)) x = v.x, y = v.y, z = v.z;
	return *this;
}
//vector-vector ops begin
bool vec3d::operator == (const vec3d &rhs) const
{
	return x == rhs.x && y == rhs.y && z == rhs.z;
}
bool vec3d::operator != (const vec3d &v) const
{
	return !(*this == v);
}
bool vec3d::operator > (const vec3d &rhs) const
{
	return this->x > rhs.x && this->y > rhs.y && this->z > rhs.z;
}
bool vec3d::operator < (const vec3d &rhs) const
{
	return this->x < rhs.x && this->y < rhs.y && this->z < rhs.z;
}
bool vec3d::operator >= (const vec3d &rhs) const
{
	return this->x >= rhs.x && this->y >= rhs.y && this->z >= rhs.z;
}
bool vec3d::operator <= (const vec3d &rhs) const
{
	return this->x >= rhs.x && this->y >= rhs.y && this->z >= rhs.z;
}

vec3d operator -(const vec3d &a) //unary negation, avoid confusion
{
	return vec3d(-a.x, -a.y, -a.z);
}
	
vec3d operator + (const vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z);
}
vec3d operator - (const vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs + -rhs);
}
vec3d operator * (const vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z);
}
vec3d operator / (const vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z);
}

vec3d operator += (vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs = lhs + rhs);
}
vec3d operator -= (vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs = lhs - rhs);
}
vec3d operator *= (vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs = (lhs * rhs));
}
vec3d operator /= (vec3d &lhs, const vec3d &rhs)
{
	return vec3d(lhs = lhs / rhs);
}
//vector-scalar ops begin
vec3d operator * (double lhs, const vec3d &rhs)
{
	return vec3d(lhs * rhs.x, lhs * rhs.y, lhs * rhs.z);
}
vec3d operator * (const vec3d &lhs, double rhs)
{
	return vec3d(lhs.x * rhs, lhs.y * rhs, lhs.z * rhs);
}
vec3d operator / (const vec3d &lhs, double rhs)
{
	return vec3d((1 / rhs) * lhs);
}
vec3d operator *= (vec3d &lhs, double rhs)
{
	return vec3d(lhs = rhs * lhs);
}
vec3d operator /= (vec3d &lhs, double rhs)
{
	return vec3d(lhs = lhs / rhs);
}
//vector-scalar ops end
double length(vec3d &v)
{
	return sqrt(v.x * v.x + v.y * v.y + v.z * v.z);
}
double dot(vec3d &a, vec3d &b)
{
	return a.x * b.x + a.y * b.y + a.z * b.z;
}
double radian_angle_between(vec3d &a, vec3d &b)
{
	double theta = dot(a, b) / (length(a) * (length(b)));
	return acos(theta);
}
vec3d floor_vec3d(vec3d &a)
{
	return vec3d(floor(a.x), floor(a.y), floor(a.z));
}
bool almost_equal(vec3d &a, vec3d &b, double epsilon)
{
	if (floor(a.x - b.x) <= epsilon && floor(a.y - b.y) <= epsilon && floor(a.z - b.z) <= epsilon)
	{
		return true;
	}
	else return false;
}
bool orthogonal(vec3d &a, vec3d &b)
{
	return dot(a, b) == 0;
}
bool orthogonal_almost(vec3d &a, vec3d &b, double epsilon)
{
	return floor(dot(a, b)) <= epsilon;
}
vec3d cross(vec3d &a, vec3d &b)
{
	 return vec3d(a.y * b.z - a.z * b.y, a.z * b.x - a.x * b.z, a.x * b.y - a.y * b.x);
}
vec3d normalize(vec3d &a)
{
	return a / length(a);
}
void print(vec3d &a)
{
	std::cout << a.x << ", " << a.y << ", " << a.z << "\n";
}
Last edited on
ne555 (10692)
> vec3d& operator = (const vec3d &rhs);
¿what's wrong with the one provided by the compiler?

> from a maths standpoint, I'm not sure how much sense the operators below make
¿then why did you make them?

> return vec3d(lhs = lhs - rhs);
¿why do you need the `vec3d' there?

1
2
3
4
5
 
vec3d operator * (const vec3d &lhs, double rhs)
{
	//return vec3d(lhs.x * rhs, lhs.y * rhs, lhs.z * rhs);
	return rhs*lhs;
}


> double length(vec3d &v)
¿why a non-const reference?
you cannot pass a temporary this way.


Also, ¿is this all in a header?
JLBorges (13770)
If you're writing a function that can be implemented as either a member or as a non-friend non-member, you should prefer to implement it as a non-member function. That decision increases class encapsulation. When you think encapsulation, you should think non-member functions. - Scott Meyers in Dr. Dobb's
http://www.drdobbs.com/cpp/how-non-member-functions-improve-encapsu/184401197?pgno=1

Canonical implementations of binary arithmetic operators:
Since for every binary arithmetic operator there exists a corresponding compound assignment operator, canonical forms of binary operators are implemented in terms of their compound assignments
'Canonical implementations' in http://en.cppreference.com/w/cpp/language/operators

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
#include <iostream>
#include <tuple>

struct vec3d
{
	explicit vec3d( double x = 0, double y = 0, double z= 0 ) : x(x), y(y), z(z) {}

	std::tuple<double,double,double> tuple() const { return std::make_tuple( x, y, z ) ; }

	vec3d& operator += ( const vec3d& that ) { x += that.x ; y += that.y ; z += that.z ;  return *this ; }
	// like-wise for other compound assignment operators

	private: double x, y, z;

	friend std::ostream& operator<< ( std::ostream& stm, const vec3d& vec )
        { return stm << "vec3d { " << vec.x << ", " << vec.y << ", " << vec.z << " }" ; }

};

bool operator== ( const vec3d& a, const vec3d& b ) { return a.tuple() == b.tuple() ; }
bool operator< ( const vec3d& a, const vec3d& b ) { return a.tuple() < b.tuple() ; }
// like-wise for other comparison operators

vec3d operator+ ( vec3d a, const vec3d& b ) { return a += b ; } // pass lhs by value, reuse compound assignment
// like-wise for other binary arithmetic operators

int main()
{
    std::cout << vec3d( 1, 2, 3 ) + vec3d( 4, 5, 6 ) << '\n' ;
}

http://coliru.stacked-crooked.com/a/001a8cb6ae8524f0
Last edited on
dhayden (5798)
Line 20: This works but it's much more common to do :
1
2
3
4
5
 
if (!(this == &v)) {
    x = v.x;
    y = v.y;
    z = v.z;
}

Operator*() and dot() do the same thing. One should be implemented in terms of the other.

All the functions from line 109-150 should be methods.

It's best to define +,-,*,/ in terms of +=, -=, *= and /=. For example:
1
2
3
 
vec3d operator+(vec3d left, const vec3d &right) {
    return left += right;
}


Change print() to a global << operator that can use any ostream:
1
2
3
 
ostream &operator <<(ostream &os const vec3d &v) {
    return os << v.x << ", " << v.y << ", " << v.z;
}
Armendhammer (11)
@ne555, this code is not in a header.
@dhayden, dot and *(vec3d, vec3d) do not do the same thing, one returns a scalar and the other returns a vector respectively.

Dhayden says that functions from 109-150 should be methods, but JLBorges disagrees. I agree with JLBorges.

I've added declarations to keep the overloads organized.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
 
namespace math3d 
{
	struct vec3d
	{
		double x, y, z;
		explicit vec3d(double x = 0, double y = 0, double z = 0) : x(x), y(y), z(z) {};
		bool operator == (const vec3d &rhs) const;
		bool operator != (const vec3d &rhs) const;
                friend std::ostream& operator << (std::ostream& ostrm, const vec3d& a)
		{
			return ostrm << "vec3d (" << a.x << ", " << a.y << ", " << a.z << ")" << "\n";
		}
	};

	vec3d operator + (const vec3d &lhs, const vec3d &rhs);
	vec3d operator - (const vec3d &lhs, const vec3d &rhs);
	vec3d operator - (const vec3d &a);
	vec3d operator * (double lhs, const vec3d &rhs);
        vec3d operator * (const vec3d &lhs, const vec3d &rhs);
	vec3d operator / (const vec3d &lhs, const vec3d &rhs);
	vec3d operator += (const vec3d &lhs, const vec3d &rhs);
	vec3d operator -= (const vec3d &lhs, const vec3d &rhs);
	vec3d operator *= (const vec3d &lhs, const vec3d &rhs);
	vec3d operator /= (const vec3d &lhs, const vec3d &rhs);

	//vector-vector ops begin
	bool vec3d::operator == (const vec3d &rhs) const
	{
		return x == rhs.x && y == rhs.y && z == rhs.z;
	}
	bool vec3d::operator != (const vec3d &a) const
	{
		return !(*this == a);
	}

	vec3d operator + (const vec3d &lhs, const vec3d &rhs)
	{
		return vec3d(lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z);
	}
	vec3d operator - (const vec3d &lhs, const vec3d &rhs)
	{
		return lhs + -rhs;
	}
	vec3d operator * (const vec3d &lhs, const vec3d &rhs)
	{
		return vec3d(lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z);
	}
	vec3d operator / (const vec3d &lhs, const vec3d &rhs)
	{
		return vec3d(lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z);
	}

	vec3d operator += (vec3d &lhs, const vec3d &rhs)
	{
		return lhs = lhs + rhs;
	}
	vec3d operator -= (vec3d &lhs, const vec3d &rhs)
	{
		return lhs = lhs - rhs;
	}
	vec3d operator *= (vec3d &lhs, const vec3d &rhs)
	{
		return lhs = lhs * rhs;
	}
	vec3d operator /= (vec3d &lhs, const vec3d &rhs)
	{
		return lhs = lhs / rhs;
	}
	//vector-scalar ops begin
	vec3d operator - (const vec3d &a) //unary negation, avoid confusion
	{
		return -1.0 * a;
	}
	vec3d operator * (double lhs, const vec3d &rhs)
	{
		return vec3d(lhs * rhs.x, lhs * rhs.y, lhs * rhs.z);
	}
	vec3d operator * (const vec3d &lhs, double rhs)
	{
		return vec3d(lhs.x * rhs, lhs.y * rhs, lhs.z * rhs);
	}
	vec3d operator / (const vec3d &lhs, double rhs)
	{
		return (1 / rhs) * lhs;
	}
	vec3d operator *= (vec3d &lhs, double rhs)
	{
		return lhs = rhs * lhs;
	}
	vec3d operator /= (vec3d &lhs, double rhs)
	{
		return lhs = lhs / rhs;
	}
	//vector-scalar ops end
	double length(const vec3d &a)
	{
		return sqrt(a.x * a.x + a.y * a.y + a.z * a.z);
	}
	double dot(const vec3d &a, const vec3d &b)
	{
		return a.x * b.x + a.y * b.y + a.z * b.z;
	}
	double radian_angle_between(const vec3d &a, const vec3d &b)
	{
		double theta = dot(a, b) / (length(a) * (length(b)));
		return acos(theta);
	}
	vec3d floor_vec3d(const vec3d &a)
	{
		return vec3d(floor(a.x), floor(a.y), floor(a.z));
	}
	bool almost_equal(const vec3d &a, const vec3d &b, double epsilon)
	{
		if (floor(a.x - b.x) <= epsilon && floor(a.y - b.y) <= epsilon && floor(a.z - b.z) <= epsilon)
		{
			return true;
		}
		else return false;
	}
	bool orthogonal(const vec3d &a, const vec3d &b)
	{
		return dot(a, b) == 0;
	}
	bool orthogonal_almost(const vec3d &a, const vec3d &b, double epsilon)
	{
		return floor(dot(a, b)) <= epsilon;
	}
	vec3d cross(const vec3d &a, const vec3d &b)
	{
		 return vec3d(a.y * b.z - a.z * b.y, a.z * b.x - a.x * b.z, a.x * b.y - a.y * b.x);
	}
	vec3d normalize(const vec3d &a)
	{
		return a / length(a);
	}
}
Last edited on
TheIdeasMan (6817)
hi,

With radian_angle_between, are you aware that the dot product equals the cosine of the angle between the 2 vectors?

With orthogonal, there are issues with that regarding the equality with floating point numbers, also with operator==. You have orthogonal_almost which on the face of it might be better, as it attempts to address those issues. But it has a floor function, how will that work if the value is say -3e-15, giving -1.0? Should you be using the std::abs function rather than std::floor in that equation, and elsewhere for the floor function? Similarly, I wonder aboutf the floor_vec3d .

Perhaps pedantically, the parameter epsilon might be a little confusing: Is it really the std::numeric_limits::epsilon or is it more of a precision value? To me, the latter might be of more use.

This is the first time I have seen anyone implement a "vector division" operation (as opposed to scalar division). What is the purpose of it? I did vector algebra at university, I don't recall any of the formulae having "vector division" like that.

With normalize I have seen that name used a lot, but I am still mystified why it is called that, as opposed to the concept of a unit vector (length 1.0). What does one name a unit vector which is normal to a plane? A normalized normal vector?
Topic archived. No new replies allowed.