Skip to main content
GameDev.net gamedev.net
🔒 Locked

C++ Vector Class

Started by induster Mar 16, 2008 at 3:55 PM 24 replies 49.1k views
Original Post
induster
induster
I have a question about the following code:

#ifndef _VECTOR
#define _VECTOR
#include <math.h>

class Vector3D
{
private:
	float x, y, z;
public:
	//default constructor
	Vector3D(float X = 0, float Y = 0, float Z = 0)
	{
		x = X;
		y = Y;
		z = Z;
	}
	~Vector3D(){};

	//calculate and return the magnitude of this vector
	float GetMagnitude()
	{
		return sqrtf(x * x + y * y + z * z);
	}

	//multiply this vector by a scalar
	Vector3D operator*(float num) const
	{
		return Vector3D(x * num, y * num, z * num);
	}

	//pass in a vector, pass in a scalar, return the product
	friend Vector3D operator*(float num, Vector3D const &vec)
	{
		return Vector3D(vec.x * num, vec.y * num, vec.z * num);
	}

	//add two vectors
	Vector3D operator+(const Vector3D &vec) const
	{
		return Vector3D(x + vec.x, y + vec.y, z + vec.z);
	}

	//subtract two vectors
	Vector3D operator-(const Vector3D &vec) const
	{
		return Vector3D(x - vec.x, y - vec.y, z - vec.z);
	}

	//normalize this vector
	void normalizeVector3D()
	{
		float magnitude = sqrtf(x * x + y * y + z * z);
		x /= magnitude;
		y /= magnitude;
		z /= magnitude;
	}
	
	//calculate and return dot product
	float dotVector3D(const Vector3D &vec) const
	{
		return x * vec.x + y * vec.y + z * vec.z;
	}

	//calculate and return cross product
	Vector3D crossVector3D(const Vector3D &vec) const
	{
		return Vector3D(y * vec.z - z * vec.y,
				z * vec.x - x * vec.z,
				x * vec.y - y * vec.x);
	}
};

#endif
1. I normally see the implementation split into a .cpp file. What are the implications of writing a class this way? More efficient? Bad code? No difference?
osmanb
osmanb
It's largely an organization vs. (potential) performance tradeoff. If you put the implementation in a .cpp file, then client code doesn't get to see the implementation. I'm not talking about security, just about organization. If you're changing the implementation of some of the functions, this is nice, because the header doesn't change, so you don't have to recompile every file that includes it.

On the other hand, putting the code into the header makes it more likely that the compiler can inline the code. For very short functions (like the ones you've shown), the compiler probably will choose to inline them, given the chance. Note that some compilers can inline even if your code is in the .cpp file (using link-time-code-generation), but if you want to ensure that any compiler (gcc, etc...) could do so, you need to leave the code in the header.
induster
induster
great explanation ty
rip-off
rip-off
Functions defined inside a class body is implicitly inline. Inline doesn't however mean anything in and of itself, the compiler is free to ignore it. It does decrease code maintainability, you have to recompile all files including it if you change it. If the code were in a source file, only the source file need be recompiled. However, this isn't a huge problem for a vector class, because they are infrequently changed.

There are some issues with your class:
0) Don't put a empty destructor. The complier will generate one for you
1) As an optimisation, you can provide a "SquaredMagnitude" function, so that comparing the relative lengths of two vectors can avoid costly sqrt calls.
2) Personally, I think "crossVector3D" and "dotCrossVector3D" are overly verbose.

Consider the usage:
Vector3 one, two;float dot = one.dot(two);Vector3 cross = two.cross(one);


Is as readable (or more so, IMO) than:
Vector3 one, two;float dot = one.dotVector3(two);Vector3 cross = two.crossVector3(one);


In my code, I have dot and cross as free functions. But that is just how I choose to do it.
induster
induster
ty for the advice. what is a free function? I don't have any experience using these functions but I'm adding the squaredMagnitude() for the day I do. I'll take your word for it.
rip-off
rip-off
One not in a class. For example, I have:

float dot(const Vector &one, const Vector &two ){    return /* ... */;}Vector cross(const Vector &one, const Vector &two ){    return /* ... */;}// usage:void example(){    Vector3 one, two;    float d = dot(one,two);    Vector3 c = cross(two,one);}
induster
induster
Do you put your dot and cross inside the vector.h?
rip-off
rip-off
Yes. I define all the operators outside the class, because none of them access private data. The only things in the class body itself are the constructors and the length() and normalise() functions.

But that is just a personal choice I made. Keeping functions inside the class body is fine.

edit:

Looking again, I see that you have made your x, y and z members private. For a simple type like a vector, it is arguable that this is unnecessary (encapsulation adds nothing). I have these variables public.
induster
induster
I have a question about normalizing vectors. The function I have changes the actual x, y, z. I would have to copy it's coordinates to another vector before normalizing it (which would overwrite it's position).

In the real world, you need to keep track of not only the normal of an object but also it's coordinates right? Or, would you have some other class keep track of where objects are in the world?
Sc4Freak
Sc4Freak
I think you have "normalisation" mixed up with "normal". In this context, normalising a vector means making it of unit length. A normal, however, is a direction vector that's perpendicular to a plane. Normals are often normalised, meaning that they are of unit length (unit length means length of 1).
MARS_999
MARS_999
I am going to throw this out, why not use a operator overloading in your class to do dot and cross.

//cross product        vec3 operator %(const vec3& v) const        {            return (vec3(y * v.z - z * v.y,			 z * v.x - x * v.z,			 x * v.y - y * v.x));        }        //dot product        vec3 operator &(const vec3& v) const        {            return (vec3(x * v.x, y * v.y, z * v.z));        }


Go ahead and use seperate cross and dot functions if you want but most likely you will be using your class objects to do the math. So this is even cleaner yet

vec3 v1, v2, result;result = v1 & v2;//dotresult = v1 % v2;//cross


But that's me.
Josef Meixner
Josef Meixner
I don't like your constructor:
//default constructorVector3D(float X = 0, float Y = 0, float Z = 0){	x = X;	y = Y;	z = Z;}

The way it is written now allows these:
Vector3D vec1;                    // 1Vector3D vec2(1.0f);              // 2Vector3D vec3(1.0f, 1.0f);        // 3Vector3D vec4(1.0f, 1.0f, 1.0f);  // 4

and 2 and 3 don't make a lot of sense and are probably an oversight when they appear. So I suggest to define two constructors, one default constructor and one taking 3 floats. Then only 1 and 4 work, the other two generate compiler warnings.
_the_phantom_
_the_phantom_
Quote:
Original post by MARS_999
I am going to throw this out, why not use a operator overloading in your class to do dot and cross.

*** Source Snippet Removed ***

Go ahead and use seperate cross and dot functions if you want but most likely you will be using your class objects to do the math. So this is even cleaner yet

*** Source Snippet Removed ***

But that's me.


That's not 'cleaner'; you've redefined what a fundimental operator in C++ does without a logical connection. Looking at the code you posted at first glance I expect the '&' to be a 'logical and' and '%' to be the modulas operation.

In short, you haven't cleaned up the code you've just obscured it. I mean, how is '&' even related to a dot product?

This is the biggest problem with C++; operator overloading abuse obsures code : don't do it.
Zakwayda
Zakwayda
Quote:
Original post by MARS_999
I am going to throw this out, why not use a operator overloading in your class to do dot and cross.

*** Source Snippet Removed ***

Go ahead and use seperate cross and dot functions if you want but most likely you will be using your class objects to do the math. So this is even cleaner yet

*** Source Snippet Removed ***

But that's me.
@The OP: I would also highly recommend not doing this.

There is no real consensus as to which operator should used for which operation. Historically, some have used * for the dot product, and some have used it for the cross product. Other variations include % (for either), ^ (for the cross product), and even & (as in the above example).

Because there is no consensus, when someone sees this in your code:
vector v1, v2;do_something(v1 * v2);
It will not be immediately clear what it means. This, however:
vector v1, v2;do_something(dot(v1, v2));
Is unambiguous.

There's more that could be said on this topic, but I've got to run so I'll leave it at that for now :)
induster
induster
The constructor:

Thanks Josef. Your post makes a lot of sense.

The overloaded dot, cross:

I really don't like the idea of using that approach. It would confuse the code more than anything. We need new ascii symbols: o and x :)
SiCrane
SiCrane
If you really want to name your own operators you can do something like:
template <typename T, typename Op>struct LtProxy {  public:    LtProxy(T r) : ref_(r) {}    T ref_;  private:      LtProxy & operator=(const LtProxy &);};double dot_product(const Vector3 & lhs, const Vector3 & rhs);const struct dot_t {} dot = {}; inline LtProxy<const Vector3 &, dot_t> operator <(const Vector3 & lhs, dot_t) {  return LtProxy<const Vector3 &, dot_t>(lhs); } inline double operator>(const LtProxy<const Vector3 &, dot_t> & lhs, const Vector3 & rhs) {   return dot_product(lhs.ref_, rhs); };Vector3 cross_product(const Vector3 & lhs, const Vector3 & rhs);const struct cross_t {} cross = {}; inline LtProxy<const Vector3 &, cross_t> operator <(const Vector3 & lhs, cross_t) {   return LtProxy<const Vector3 &, cross_t>(lhs); } inline Vector3 operator>(const LtProxy<const Vector3 &, cross_t> & lhs, const Vector3 & rhs) {   return cross_product(lhs.ref_, rhs); };

This allows you to write code like a b and a b
MARS_999
MARS_999
Quote:
Original post by phantom
Quote:
Original post by MARS_999
I am going to throw this out, why not use a operator overloading in your class to do dot and cross.

*** Source Snippet Removed ***

Go ahead and use seperate cross and dot functions if you want but most likely you will be using your class objects to do the math. So this is even cleaner yet

*** Source Snippet Removed ***

But that's me.


That's not 'cleaner'; you've redefined what a fundimental operator in C++ does without a logical connection. Looking at the code you posted at first glance I expect the '&' to be a 'logical and' and '%' to be the modulas operation.

In short, you haven't cleaned up the code you've just obscured it. I mean, how is '&' even related to a dot product?

This is the biggest problem with C++; operator overloading abuse obsures code : don't do it.


Like I said IMO, not your opinion.
Zakwayda
Zakwayda
Quote:
Original post by MARS_999
Quote:
Original post by phantom
Quote:
Original post by MARS_999
I am going to throw this out, why not use a operator overloading in your class to do dot and cross.

*** Source Snippet Removed ***

Go ahead and use seperate cross and dot functions if you want but most likely you will be using your class objects to do the math. So this is even cleaner yet

*** Source Snippet Removed ***

But that's me.


That's not 'cleaner'; you've redefined what a fundimental operator in C++ does without a logical connection. Looking at the code you posted at first glance I expect the '&' to be a 'logical and' and '%' to be the modulas operation.

In short, you haven't cleaned up the code you've just obscured it. I mean, how is '&' even related to a dot product?

This is the biggest problem with C++; operator overloading abuse obsures code : don't do it.


Like I said IMO, not your opinion.
IMO (haha), not all opinions regarding programming style and technique are created equal.

Some issues (such as bracket placement) are arguably subjective and subjective only. Others (such as, say, whether or not it's appropriate to include using directives or declarations in header files) are pretty cut and dried.

For example, you could easily say, "In my opinion, there's nothing wrong with putting 'using namespace std' at the top of your header files; I do this in all my projects, and I've never had a problem." In so doing, you've just stated an opinion, and opinions can't really be 'wrong', can they?

Well, maybe, but I would argue that some opinions are more valid than others. My own opinion is that putting using statements in header files is a bad idea. Furthermore, I would argue that my opinion is 'more right' than the hypothetical opinion stated in the preceding paragraph.

So, coming back to overloaded operators for vector multiplication, I don't know that there's a clear right or wrong here (as there is, I believe, with the 'using statement' issue). However, I think the opinion that such overloads should be avoided is more right than the opinion that their use is ok.

Now, you'll have no trouble finding example code that backs up your position. You can find all sorts of math libraries (in books, online, and even in the code bases for AAA games) that provide all kinds of weird overloads for various types of math classes. I've seen just about every operator used in this way (!, %, ^, &, *, /, etc.) for everything from dot and cross products, to vector normalization, to vector length.

But therein lies part of the problem. Each of these math libraries uses different operators to mean different things. As such, every time you're faced with an unfamiliar math library, you have to re-learn what the operators mean.

A related problem is that the meaning of the operators must be deduced from the context. A few examples:
float d = v1*v2;// That's not too bad. We know from experience that the variable names// 'v1' and 'v2' probably refer to vectors, and we know that the return// value is a scalar, so this is probably the dot product.vector v = v1*v2;// Using similar logic, this is probably the cross product. But, it could// also be component-wise multiplication (I've seen the * operator overloaded// this way before). So, off to the appropriate header or source file to figure // out what's going on here.// Now, what if a) the assigned-to variable was declared elsewhere, and// b) we don't have such friendly variable names:a = b*c;// Who knows? It could be scalar multiplication, or scalar-vector// multiplication, or component-wise vector multiplication, or the// dot product, or the cross product. Granted, even with more// conservative use of overloading there's still some ambiguity here// (which is one of the reasons that good variable names are important),// but still much less so than if we were to overload for vector-vector// multiplication.some_function(v1*v2);// We're kind of stuck here too. Again, this could be the dot product,// component-wise multiplication, or the cross product. We can narrow this// down by looking at the argument type for some_function(), but we// might have to go digging even farther than that, depending on what// the * operator is overloaded for.a = !b * ((a%c) & d);// Good luck with that.
This post is getting pretty long, so I'll address one more issue and call it good.

Something you'll find in many programming and style guides for languages that support operator overloading is this: the function assigned to an overloaded operator should in some way relate to its built-in function.

Using this logic, overloading the * operator for vector products is defensible (although still confusing and ambiguous). However, most of the other overloads you might use (%, ^, &) have built-in meanings that are completely different than the functions they would most likely be assigned in the context of a math library.

Again, there are plenty of math libraries out there that incorporate these types of operator overloads. However, IMO, if you're creating a new math library from scratch there's little reason to go down that road.
MARS_999
MARS_999
As always JYK, I appreciate what you have to say. I agree when you work in a large project or plan on selling some library standards should be implemented. But for someone who is doing this as a hobby or plans on using it for themselves use whatever flips your trigger.
Zakwayda
Zakwayda
Quote:
But for someone who is doing this as a hobby or plans on using it for themselves use whatever flips your trigger.
Yup, there's definitely some truth there :)

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.