Need quick C++ code review(150 lines),please

sidney.harrell · 06-08-2007, 08:23 PM

I had to take a break from "Teach Yourself C++ in 21 days" at about day 9 to flesh out some ideas that had been floating around, and boy did it get complicated quick. I thought I would just declare a point class and then a line class using the point class as endpoints and I got derailed by "well, what is a line, really? can't you define it by one endpoint and a vector?" So then I had to go back and define a vector class, then overload addition, subtraction, and assignment operators, then ...blah,blah,blah.
Anyway, I've got about 150 lines so far and I thought I should have someone experienced look it over for any obvious design flaws. Everyone should be able to access it here:
http://learning-cplusplus.googlecode...runk/extra.cpp
Let me know if it would be more appropriate to post the whole thing here. The last specific problem I had was here:

Quote:

Vector Vector:

perator+ (Vector & rhs) {
Point origin(0,0);
Point plhs, prhs;
plhs = (*this)+origin;
prhs = (rhs)+origin;
return Vector(plhs+prhs);
}

I had to drop the const from the Vector reference in the parameter list because the compiler had a problem with the line:
prhs = (rhs)+origin;
Sorry about the lack of comments, let me know what's not obvious and I can easily through some in and update it with svn.

alunduil · 06-08-2007, 08:30 PM

I've glanced at the code, and even compiled it.

You might want to change #include <iostream.h> to #include <iostream>. The latter has some newer things for C++.

Also later in the file you don't scope your cout, and endl statements with std::. Just thought I'd mention those couple of things in case you'd missed them.

Other than though, I don't know what the program is supposed to do, and can't tell if it's doing it or not.

What exactly is the problem you are having with this code?

Regards,

Alunduil

P.S. For a sampling of my C++ check out: http://svn.alunduil.com/svn/graph/trunk

sidney.harrell · 06-08-2007, 08:50 PM

I haven't gotten to scope yet in the text I'm following, and I just looked ahead and I don't think it's even mentioned, but then again it's probably really simplified to try and fit as much into the "21 days" as possible.
http://guides.oernii.sk/c++/index.htm
It seems to be a bit better than "C++ for dummies", which I have the pdf of.
I've just been using main() as a test bed, to check the classes and methods and make sure they work as expected. I'm going to try to exhaust what can be done with points and vectors before I move on to something as complicated as a line, which judging by how the code required for vectors ballooned on me, will probably be a thousand lines and bring up issues I hadn't considered before.

Millenniumman · 06-08-2007, 09:06 PM

I'd recommend that you decide on something to do, rather than try to do everything with a given concept. Your code will be a lot smaller, and you'll probably learn more. It's also more in line with practical coding.

The thing is, even if you have no mistakes, unnecessary code could be considered a design flaw in and of itself. And it's impossible to tell what is necessary or unnecessary when you aren't trying to do anything.

But if you just like exploring these concepts, that's fine as well.

sidney.harrell · 06-08-2007, 10:14 PM

I think it's the Ableman, Sussman MIT SICP series that's got me. Lisp is like programming stripped down to core logical essentials, and I think it's about halfway through the series they show that you can use any language that forms a closed loop to construct any other language, and that the point of any language is not the language itself, but what you can construct with it. The problem with Lisp is that it's sooo constrained that it takes forever to write anything practical, IMHO.
I guess I'd just like to become proficient at translating the math that I know (my BS is in physics), into code before I try to go back to grad school for computational biophysics. I got sidetracked into a trade for the last 9 years (how time flys), but 18 months ago I discovered podcasts which opened up a whole new world of Linux, and Floss, and Web2.0, and what kind of content can I generate? Well once upon a time I taught myself to code in Pascal and Fortran, so what language should I be writing in now? Well the linux kernel is written in C, so I studied C. But when I went to look at libraries for displaying data I went to OpenGL and GTK+ and Glade, and I found all this stuff about event catching and passing pointers to functions as parameters. Stuff I kind of suspect is more C++ than C, or at least introductory C, so I'm studying C++. The goal I have in mind is to be able to display arbitrary 2 or 3 dimensional data, which means interfacing with the graphics libraries.

dmail · 06-09-2007, 02:55 AM

Quote:

...translating the math that I know (my BS is in physics), into code...

Really? I find it strange that you represent a vector as a radian and magnitude, what's the reasoning for this and from what is the angle taken ie what does it relate to, is it from a normal [0,1]?

Quote:

The goal I have in mind is to be able to display arbitrary 2 or 3 dimensional data, which means interfacing with the graphics libraries.

While your code can represent a vector in 2d it can not in 3 or more dimensions. Its true to say a vector has a length and a direction, but its also true to say a 2d vector is four numbers two real and two imaginary. These graphic libraries, how would you say they represent vectors? as a radian and magnitude?

sidney.harrell · 06-09-2007, 07:32 AM

Just using the standard vector notation, where coordinate pair x=1, y=0 would be angle=0, length=1 and coordinate pair x=0, y=1 would be angle=pi/2, length=1. I hadn't considered feeding the vector representation to the graphics library yet, although I could see a senario later where you would have a three dimensional field of three dimensional vectors you need to visualize, but right now I would be happy to be able to take a list of coordinate pairs and plot them. I know I could do this staticly by having the data generating code output to a file, then feed that file into gnuplot, but what I would like to do is move to a dynamic model, where the data generating code can update the data file and the changes would be displayed in real time. It's the kind of thing that's probably trivial to someone who knows game programming.
The vector representation does make certain problems easier. For instance, to find the midpoint of a line you could take the endpoint and vector representation and scale the vector by 0.5
I don't want to take on 3d yet until I've fleshed out the 2d, but it should just be a matter of building up the structures from the 2d. A 3d coordinate would just add a z coordinate, a 3d vector would just add an angle of rotation from the x-y plane, etc.

dmail · 06-09-2007, 08:48 AM

Quote:

Just using the standard vector notation, where coordinate pair x=1, y=0 would be angle=0, length=1 and coordinate pair x=0, y=1 would be angle=pi/2, length=1.

I have never come across this standard notation and it is defiantly not something used in computer games, it is more akin to Euler angles. Standard notation to me is [1,0] and [0,1] as per your examples and mag is equal to |m| = ( x^2 + y^2 )^1/2

Quote:

The vector representation does make certain problems easier..

As I sure you are aware a vector is a special matrix which has only one column (or row) yet each to there own. I suggest you read some material on how everyone else represents these in computer games, how libraries expect the values and how models etc supply them:
http://www.gamedev.net/reference/lis...goryid=28#260/

Here is an example of the sort of class I use

Code:

namespace MATHS
{

class Vector3
{
public:
	//constructors
	Vector3();
	explicit Vector3(float x_,float y_,float z_);
	explicit Vector3(Vector3 const& rhs);
	explicit Vector3(float const* rhs);

	//destructor
	~Vector3();

	//assignment operators
	inline Vector3& operator = (Vector3 const& rhs);
	inline Vector3& operator -= (Vector3 const& rhs);
	inline Vector3& operator += (Vector3 const & rhs);
	inline Vector3& operator *=(float const& scalar);
	inline Vector3& operator /= (float const& scalar);

	//check opertors
	inline bool operator == (Vector3 const & rhs)const;
	inline bool operator != (Vector3 const& rhs)const;

	inline Vector3 operator + (Vector3 const & rhs)const;
	inline Vector3 operator -(Vector3 const& rhs)const;
	inline Vector3 operator * (float const& scalar)const;
	inline Vector3 operator /(float const& scalar);

	//friend functions
	friend Vector3 operator * ( float f, Vector3 const& rhs );
	friend  float operator * (Vector3 const& lhs,Vector3 const& rhs);
	friend Vector3 normalise(Vector3 & v);
	friend float magnitude_sqr(Vector3 const& v);
	friend float magnitude(Vector3 const& v);
	friend float angle_between(Vector3 const& lhs,Vector3 const& rhs);
	friend Vector3 cross(Vector3 const& lhs,Vector3 const& rhs);


	inline Vector3 invert()const;
	//cast operators
	operator float* ()
	{
		return &x;
	}
	operator float const* () const
	{
		return &x;
	}

private:
	float x,y,z;
};

}//end of namespace MATHS



//constuctors
MATHS::Vector3::Vector3() : x(0.0f),y(0.0f),z(0.0f){}

MATHS::Vector3::Vector3(float x_,float y_,float z_) : x(x_),y(y_),z(z_){}

MATHS::Vector3::Vector3(MATHS::Vector3 const& rhs) : x(rhs.x), y(rhs.y), z(rhs.z){}

MATHS::Vector3::Vector3(float const* rhs) : x(rhs[0]), y(rhs[1]), z(rhs[2]){}	

//desturctor
MATHS::Vector3::~Vector3(){}	

//assignment operators
MATHS::Vector3& MATHS::Vector3::operator = (MATHS::Vector3 const& rhs)
{
	x = rhs.x; y = rhs.y; z = rhs.z;
	return *this;
}

MATHS::Vector3& MATHS::Vector3::operator *=(float const& scalar)
{
	*this = *this * scalar;
	return *this;
}

MATHS::Vector3& MATHS::Vector3::operator -=(MATHS::Vector3 const& rhs)
{
	*this = *this - rhs;
	return *this;
}

MATHS::Vector3& MATHS::Vector3::operator+=(MATHS::Vector3 const & rhs)
{
	x += rhs.x;
	y += rhs.y;
	z += rhs.z;
	return *this;
}

MATHS::Vector3& MATHS::Vector3::operator /=(float const& scalar)
{
	//prevent division by zero and one
	if(MATHS::equal(scalar,0.0f) || MATHS::equal(scalar,1.0f)){return *this;}
	//mutliplication is quicker than division
	float f = 1.0f / scalar;
	x *= f;
	y *= f;
	z *= f;
	return *this;
}
....