• Advertisement
Sign in to follow this  

C++ Vector Class

This topic is 3621 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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?

Share this post


Link to post
Share on other sites
Advertisement
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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);
}

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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;//dot
result = v1 % v2;//cross



But that's me.

Share this post


Link to post
Share on other sites
I don't like your constructor:

//default constructor
Vector3D(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; // 1
Vector3D vec2(1.0f); // 2
Vector3D vec3(1.0f, 1.0f); // 3
Vector3D 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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 :)

Share this post


Link to post
Share on other sites
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 :)

Share this post


Link to post
Share on other sites
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 <dot> b and a <cross> b

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 :)

Share this post


Link to post
Share on other sites
I'm actually not certain about this, but doesn't operator precedence still apply to overloaded operators?

If so, it adds a whole other can of worms to choosing symbols that not only make sense, but preserve proper precedence too. I can see incorrect precedence ordering being the source of rather nefarious bugs...

Share this post


Link to post
Share on other sites
Quote:
Original post by Ravyne
I'm actually not certain about this, but doesn't operator precedence still apply to overloaded operators?


It does, or better yet, should (with C++ you never know).

It's still a can of worms.

Share this post


Link to post
Share on other sites
Quote:
Original post by 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.


The only problem with this is when you take the accepted meaning of something, change it fundimentally and THEN ask for help from other programmers they aren't going to have a clue wtf is going on.

End of the day I honestly don't care, but at the same time I strongly recommend against anyone using such a system because it's insane and you effectively cut anyone else out from being able to help you out with problems in your code.

Also, you never said 'imo' you stated "So this is even cleaner yet", a definitative statement which, as statements go, is incorrect, as it's not 'cleaner' for the stated reason that it goes against already accepeted concepts and usage for the operators (one of the unwritten rules of operator overloading is 'dont change the meaning' [paraphased] as they should really do what people expect).

"imo" is no cover for simply bad advice...

Share this post


Link to post
Share on other sites
Quote:
Original post by phantom
Quote:
Original post by 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.


The only problem with this is when you take the accepted meaning of something, change it fundimentally and THEN ask for help from other programmers they aren't going to have a clue wtf is going on.

End of the day I honestly don't care, but at the same time I strongly recommend against anyone using such a system because it's insane and you effectively cut anyone else out from being able to help you out with problems in your code.

Also, you never said 'imo' you stated "So this is even cleaner yet", a definitative statement which, as statements go, is incorrect, as it's not 'cleaner' for the stated reason that it goes against already accepeted concepts and usage for the operators (one of the unwritten rules of operator overloading is 'dont change the meaning' [paraphased] as they should really do what people expect).

"imo" is no cover for simply bad advice...


Maybe you should read more carefully. Go back and read the ending of my post. It says... "But that's me." Correct!

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement