Can you spot anything wrong with this class/structure?

J

Jim Langston

I use a game engine using MSVC++ .net 2003 and have no problems. Some users
of DevC++ who use the same engine crash at times when a copy of this
structure is the return variable. I don't have access to the code that is
crashing, but the developer has been changing functions that return the
structure to return something else because of this problem.

Personally, I don't see anything wrong with this structure and can't imagine
why it would be crashing in DevC++.

Aside from the all caps used outside of a macro which is frowned upon, can
you spot anything wrong with this strucutre? It looks perfectly legitimite
to me. ( I know the consructor should be using an initializer list and I've
suggested that ).

struct JVEC2
{
JVEC2(float x=0,float y=0){this->x=x;this->y=y;}
// Supported operators
JVEC2 operator+(JVEC2 a) const {JVEC2 temp;temp.x = x + a.x;temp.y = y +
a.y;return temp;}
JVEC2 operator-(JVEC2 a) const {JVEC2 temp;temp.x = x - a.x;temp.y = y -
a.y;return temp;}
JVEC2 operator*(JVEC2 a) const {JVEC2 temp;temp.x = x * a.x;temp.y = y *
a.y;return temp;}
JVEC2 operator/(JVEC2 a) const {JVEC2 temp;temp.x = x / a.x;temp.y = y /
a.y;return temp;}
JVEC2 operator+(float a) const {JVEC2 temp;temp.x = x + a;temp.y = y +
a;return temp;}
JVEC2 operator-(float a) const {JVEC2 temp;temp.x = x - a;temp.y = y -
a;return temp;}
JVEC2 operator*(float a) const {JVEC2 temp;temp.x = x * a;temp.y = y *
a;return temp;}
JVEC2 operator/(float a) const {JVEC2 temp;temp.x = x / a;temp.y = y /
a;return temp;}
JVEC2 operator+=(JVEC2 a) {x += a.x;y += a.y;return *this;}
JVEC2 operator*=(JVEC2 a) {x *= a.x;y *= a.y;return *this;}
JVEC2 operator-=(JVEC2 a) {x -= a.x;y -= a.y;return *this;}
JVEC2 operator/=(JVEC2 a) {x /= a.x;y /= a.y;return *this;}
JVEC2 operator+=(float a) {x += a;y += a;return *this;}
JVEC2 operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2 operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2 operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (JVEC2 a) const {if (x == a.x && y == a.y) return true;
else return false;}
bool operator != (JVEC2 a) const {if (x != a.x || y != a.y) return true;
else return false;}
JVEC2 operator + () const {JVEC2 t;t.x=+x;t.y=+y;return t;}
JVEC2 operator - () const {JVEC2 t;t.x=-x;t.y=-y;return t;}
// data
float x,y;
};
 
V

Victor Bazarov

Jim said:
I use a game engine using MSVC++ .net 2003 and have no problems.
Click to expand...

Doesn't really sound like it, since you're posting this...
Some users of DevC++ who use the same engine crash at times when a
copy of this structure is the return variable. I don't have access
to the code that is crashing, but the developer has been changing
functions that return the structure to return something else because
of this problem.
Personally, I don't see anything wrong with this structure and can't
imagine why it would be crashing in DevC++.
Click to expand...

DevC++ is an IDE. How can a struct crash an IDE?

Anyway, without seeing the code that does crash, I can only make
a few suggestions, see below.
Aside from the all caps used outside of a macro which is frowned
upon, can you spot anything wrong with this strucutre? It looks
perfectly legitimite to me. ( I know the consructor should be using
an initializer list and I've suggested that ).

struct JVEC2
{
JVEC2(float x=0,float y=0){this->x=x;this->y=y;}
Ouch.

// Supported operators
JVEC2 operator+(JVEC2 a) const {JVEC2 temp;temp.x = x + a.x;temp.y
= y + a.y;return temp;}
Click to expand...

Ugh... Yechhh! Here is the pattern you need to follow:

JVEC2 operator+(JVEC2 const& a) const {
return JVEC2(x + a.x, y + a.y);
}

Rewrite all non-compound operators.
JVEC2 operator-(JVEC2 a) const {JVEC2 temp;temp.x = x - a.x;temp.y
= y - a.y;return temp;}
JVEC2 operator*(JVEC2 a) const {JVEC2 temp;temp.x = x * a.x;temp.y
= y * a.y;return temp;}
JVEC2 operator/(JVEC2 a) const {JVEC2 temp;temp.x = x / a.x;temp.y
= y / a.y;return temp;}
JVEC2 operator+(float a) const {JVEC2 temp;temp.x = x + a;temp.y =
y + a;return temp;}
JVEC2 operator-(float a) const {JVEC2 temp;temp.x = x - a;temp.y =
y - a;return temp;}
JVEC2 operator*(float a) const {JVEC2 temp;temp.x = x * a;temp.y =
y * a;return temp;}
JVEC2 operator/(float a) const {JVEC2 temp;temp.x = x / a;temp.y =
y / a;return temp;}
JVEC2 operator+=(JVEC2 a) {x += a.x;y += a.y;return *this;}
Click to expand...

Ugh... Compound assignments should return a *reference*! And
they should take a ref to const!
JVEC2 operator*=(JVEC2 a) {x *= a.x;y *= a.y;return *this;}
JVEC2 operator-=(JVEC2 a) {x -= a.x;y -= a.y;return *this;}
JVEC2 operator/=(JVEC2 a) {x /= a.x;y /= a.y;return *this;}
JVEC2 operator+=(float a) {x += a;y += a;return *this;}
JVEC2 operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2 operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2 operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (JVEC2 a) const {if (x == a.x && y == a.y) return
true; else return false;}
bool operator != (JVEC2 a) const {if (x != a.x || y != a.y) return
true; else return false;}
JVEC2 operator + () const {JVEC2 t;t.x=+x;t.y=+y;return t;}
JVEC2 operator - () const {JVEC2 t;t.x=-x;t.y=-y;return t;}
Click to expand...

Yikes! Same here

JVEC2 operator +() const { return *this; } // unary plus does
// nothing for floats

JVEC2 operator -() const { return JVEC2(-x, -y); }
// data
float x,y;
};
Click to expand...

V
 
J

Jim Langston

Victor Bazarov said:
Doesn't really sound like it, since you're posting this...
Click to expand...

I have no problems with the code, but people who compile with DevC++ have
problems when the executable crashes.
DevC++ is an IDE. How can a struct crash an IDE?

Anyway, without seeing the code that does crash, I can only make
a few suggestions, see below.


Ouch.
Click to expand...

Yeah, I agree. Been trying to get him to change this.
Ugh... Yechhh! Here is the pattern you need to follow:

JVEC2 operator+(JVEC2 const& a) const {
return JVEC2(x + a.x, y + a.y);
}
Click to expand...

Hmm.. now that he has the constructor, yeah, he can do this. It woudl
simplify the code, but don't know if it could prevent a crash.
Rewrite all non-compound operators.


Ugh... Compound assignments should return a *reference*! And
they should take a ref to const!
Click to expand...

What would they return a reference to though? If it's the temp, won't that
disappear? Which is correct:
JVEC2& operator-(const JVEC2 a) {x = x - a.x; y = y - a.y; return this;}
or
JVEC2& operator-(const JVEC2 a) const {JVEC2 temp;temp.x = x - a.x;temp.y
= y - a.y;return temp;}
 
J

Jim Langston

Victor Bazarov said:
Doesn't really sound like it, since you're posting this...


DevC++ is an IDE. How can a struct crash an IDE?

Anyway, without seeing the code that does crash, I can only make
a few suggestions, see below.


Ugh... Yechhh! Here is the pattern you need to follow:

JVEC2 operator+(JVEC2 const& a) const {
return JVEC2(x + a.x, y + a.y);
}

Rewrite all non-compound operators.


Ugh... Compound assignments should return a *reference*! And
they should take a ref to const!


Yikes! Same here

JVEC2 operator +() const { return *this; } // unary plus does
// nothing for floats

JVEC2 operator -() const { return JVEC2(-x, -y); }
Click to expand...

Okay, how does this look?

struct JVEC2
{
JVEC2(float x=0,float y=0): x(x), y(y) {}
// Supported operators
JVEC2 operator+(JVEC2 a) const { return JVEC2( x + a.x, y + a.y ); }
JVEC2 operator-(JVEC2 a) const { return JVEC2( x - a.x, y - a.y ); }
JVEC2 operator*(JVEC2 a) const { return JVEC2( x * a.x, y * a.y ); }
JVEC2 operator/(JVEC2 a) const { return JVEC2( x / a.x, y / a.y ); }
JVEC2 operator+(float a) const { return JVEC2( x + a, y + a ); }
JVEC2 operator-(float a) const { return JVEC2( x - a, y - a ); }
JVEC2 operator*(float a) const { return JVEC2( x * a, y * a ); }
JVEC2 operator/(float a) const { return JVEC2( x / a, y / a ); }
JVEC2& operator+=(const JVEC2& a) {x += a.x;y += a.y;return *this;}
JVEC2& operator*=(const JVEC2& a) {x *= a.x;y *= a.y;return *this;}
JVEC2& operator-=(const JVEC2& a) {x -= a.x;y -= a.y;return *this;}
JVEC2& operator/=(const JVEC2& a) {x /= a.x;y /= a.y;return *this;}
JVEC2& operator+=(float a) {x += a;y += a;return *this;}
JVEC2& operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2& operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2& operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (JVEC2 a) const {if (x == a.x && y == a.y) return true;
else return false;}
bool operator != (JVEC2 a) const {if (x != a.x || y != a.y) return true;
else return false;}
JVEC2 operator + () const {return JVEC2( +x, +y ); }
JVEC2 operator - () const {return JVEC2( -x, -y ); }
// data
float x,y;
};
 
I

Ian Collins

Jim said:
Okay, how does this look?

struct JVEC2
{
JVEC2(float x=0,float y=0): x(x), y(y) {}
// Supported operators
JVEC2 operator+(JVEC2 a) const { return JVEC2( x + a.x, y + a.y ); }
JVEC2 operator-(JVEC2 a) const { return JVEC2( x - a.x, y - a.y ); }
JVEC2 operator*(JVEC2 a) const { return JVEC2( x * a.x, y * a.y ); }
JVEC2 operator/(JVEC2 a) const { return JVEC2( x / a.x, y / a.y ); }
Click to expand...

These should take a const reference as well.
JVEC2 operator+(float a) const { return JVEC2( x + a, y + a ); }
JVEC2 operator-(float a) const { return JVEC2( x - a, y - a ); }
JVEC2 operator*(float a) const { return JVEC2( x * a, y * a ); }
JVEC2 operator/(float a) const { return JVEC2( x / a, y / a ); }
JVEC2& operator+=(const JVEC2& a) {x += a.x;y += a.y;return *this;}
JVEC2& operator*=(const JVEC2& a) {x *= a.x;y *= a.y;return *this;}
JVEC2& operator-=(const JVEC2& a) {x -= a.x;y -= a.y;return *this;}
JVEC2& operator/=(const JVEC2& a) {x /= a.x;y /= a.y;return *this;}
JVEC2& operator+=(float a) {x += a;y += a;return *this;}
JVEC2& operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2& operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2& operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (JVEC2 a) const {if (x == a.x && y == a.y) return true;
else return false;}
bool operator != (JVEC2 a) const {if (x != a.x || y != a.y) return true;
else return false;}
Click to expand...

So should these. A cleaner form would be

bool operator==( const JVEC2& a) const {return (x == a.x && y == a.y); }
 
J

Jim Langston

Ian Collins said:
These should take a const reference as well.


So should these. A cleaner form would be

bool operator==( const JVEC2& a) const {return (x == a.x && y == a.y); }
Click to expand...

Ooops, you're right, missed those.

struct JVEC2
{
JVEC2(float x=0,float y=0): x(x), y(y) {}
// Supported operators
JVEC2 operator+(const JVEC2& a) const { return JVEC2( x + a.x, y +
a.y ); }
JVEC2 operator-(const JVEC2& a) const { return JVEC2( x - a.x, y -
a.y ); }
JVEC2 operator*(const JVEC2& a) const { return JVEC2( x * a.x, y *
a.y ); }
JVEC2 operator/(const JVEC2& a) const { return JVEC2( x / a.x, y /
a.y ); }
JVEC2 operator+(float a) const { return JVEC2( x + a, y + a ); }
JVEC2 operator-(float a) const { return JVEC2( x - a, y - a ); }
JVEC2 operator*(float a) const { return JVEC2( x * a, y * a ); }
JVEC2 operator/(float a) const { return JVEC2( x / a, y / a ); }
JVEC2& operator+=(const JVEC2& a) {x += a.x;y += a.y;return *this;}
JVEC2& operator*=(const JVEC2& a) {x *= a.x;y *= a.y;return *this;}
JVEC2& operator-=(const JVEC2& a) {x -= a.x;y -= a.y;return *this;}
JVEC2& operator/=(const JVEC2& a) {x /= a.x;y /= a.y;return *this;}
JVEC2& operator+=(float a) {x += a;y += a;return *this;}
JVEC2& operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2& operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2& operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (const JVEC2& a) const {if (x == a.x && y == a.y)
return true; else return false;}
bool operator != (const JVEC2& a) const {if (x != a.x || y != a.y)
return true; else return false;}
JVEC2 operator + () const {return JVEC2( +x, +y ); }
JVEC2 operator - () const {return JVEC2( -x, -y ); }
// data
float x,y;
};
 
C

Clark Cox

Ooops, you're right, missed those.

struct JVEC2
{
JVEC2(float x=0,float y=0): x(x), y(y) {}
// Supported operators
JVEC2 operator+(const JVEC2& a) const { return JVEC2( x + a.x, y +
a.y ); }
JVEC2 operator-(const JVEC2& a) const { return JVEC2( x - a.x, y -
a.y ); }
JVEC2 operator*(const JVEC2& a) const { return JVEC2( x * a.x, y *
a.y ); }
JVEC2 operator/(const JVEC2& a) const { return JVEC2( x / a.x, y /
a.y ); }
JVEC2 operator+(float a) const { return JVEC2( x + a, y + a ); }
JVEC2 operator-(float a) const { return JVEC2( x - a, y - a ); }
JVEC2 operator*(float a) const { return JVEC2( x * a, y * a ); }
JVEC2 operator/(float a) const { return JVEC2( x / a, y / a ); }
JVEC2& operator+=(const JVEC2& a) {x += a.x;y += a.y;return *this;}
JVEC2& operator*=(const JVEC2& a) {x *= a.x;y *= a.y;return *this;}
JVEC2& operator-=(const JVEC2& a) {x -= a.x;y -= a.y;return *this;}
JVEC2& operator/=(const JVEC2& a) {x /= a.x;y /= a.y;return *this;}
JVEC2& operator+=(float a) {x += a;y += a;return *this;}
JVEC2& operator-=(float a) {x -= a;y -= a;return *this;}
JVEC2& operator*=(float a) {x *= a;y *= a;return *this;}
JVEC2& operator/=(float a) {x /= a;y /= a;return *this;}
bool operator == (const JVEC2& a) const {if (x == a.x && y == a.y)
return true; else return false;}
bool operator != (const JVEC2& a) const {if (x != a.x || y != a.y)
return true; else return false;}
Click to expand...
These operators are written a little strangely, if I were writing them, I'd do:
bool operator==(const JVEC2& a) const { return (x == a.x && y == a.y); }
bool operator!=(const JVEC2& a) const { return !operator==(a); }
JVEC2 operator + () const {return JVEC2( +x, +y ); }
Click to expand...
For this, I'd just do:
JVEC2 operator + () const { return *this; }
 
J

James Kanze

What would they return a reference to though?
Click to expand...

*this. Because that's what the built-in versions do.

This could even be the source of your problem, e.g. if someone
is using the return value to initialize a reference, either in a
context which doesn't extend the lifetime, or if that reference
is then used to initialize some other references. That results
in undefined behavior; depending on how and when the resulting
reference is used, it might crash, or it might work, depending
on the compiler (and probably compiler options, like the level
of optimization).
If it's the temp, won't that
disappear? Which is correct:
JVEC2& operator-(const JVEC2 a) {x = x - a.x; y = y - a.y; return this;}
or
JVEC2& operator-(const JVEC2 a) const {JVEC2 temp;temp.x = x - a.x;temp..y
= y - a.y;return temp;}
Click to expand...

That's not a compound assignment. The binary operators should
return values; the compound assignment operators references to
this.

Given that you have implicit conversions as well, the binary
operators should probably not be members, but rather free
functions, so you can write things like 1.0 + aJvec2 as well as
aJvec2 + 1.0.
 
J

James Kanze

Jim Langston wrote:
Click to expand...
These should take a const reference as well.
Click to expand...

The object is small enough that it probably doesn't really
matter (but a const reference would be more idiomatic). On the
other hand, they definitly should be free functions, not
members.
 
R

Richard Herring

Victor said:
Doesn't really sound like it, since you're posting this...


DevC++ is an IDE. How can a struct crash an IDE?

Anyway, without seeing the code that does crash, I can only make
a few suggestions, see below.


Ugh... Yechhh! Here is the pattern you need to follow:

JVEC2 operator+(JVEC2 const& a) const {
return JVEC2(x + a.x, y + a.y);
}

Rewrite all non-compound operators.
Click to expand...

You might also consider rewriting them to use the compound ones, to
guarantee consistency...

JVEC2 operator+(JVEC2 const & a) const
{ JVEC2 temp(*this); temp += a; return temp; }

.... or even rewriting them as friend binary functions, to make both
sides of the operator candidates for implicit conversions, if there are
any...

friend JVEC2 operator+(JVEC2 const & a, JVEC2 const & b)
{ JVEC2 temp(a); temp += b; return temp; }
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

how to use functions of base class 1
Can anyone help me whats wrong with this 2
Drawing missing in bitmap in a pure C win32 program 4
Weird Behavior with Rays in C and OpenGL 4
STL set (compilartion error) how to resolve this 3
In 3 moves, where can the knight go? 13
K&R p 130 29
what may cause compile error (expected nested-name-specifier before 'Policy-T')? 2

Members online

Total: 43 (members: 1, guests: 42)
Robots: 663

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,057
Latest member
KetoBeezACVGummies

Latest Threads

Top