Angle class - how is my code ?

[Darren Grant]

Hi there,

I've attempted to implement an Angle class. An Angle is a subset of an
integer, where the range is [0,360). All other operations should be
permitted.

The code works, I think... except (for example) a = b + 10;
needs to be a = b + (Angle) 10;

Could some kind soul comment on my code and show me how it could be
done better?

class Angle {
friend std::ostream& operator<<(std::ostream&, const Angle&) {
os << s.theta;
return os;
}

public:
Angle() : theta(0) { };
Angle(int t) : theta(t) { };
Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360;
return *this;
}

Angle& operator+=(const Angle& a) {
theta = (theta + a) % 360;
if (theta < 0)
theta = 360 - abs(theta);
return *this;
}

Angle& operator-=(const Angle& a) {
Angle t = a % 360;
theta = theta - t;
if (theta < 0)
theta = 360 - abs(theta);
return *this;
}

operator int() const {
return theta;
}

private:
int theta;
};

Angle operator-(const Angle& l, const Angle& r) {
Angle a = l;
a -= r;
return a;
}

Angle operator+(const Angle& l, const Angle& r) {
Angle a = l;
a += r;
return a;
}

Many thanks,
Darren

[osmium]

Darren Grant writes:

I've attempted to implement an Angle class. An Angle is a subset of an
integer, where the range is [0,360). All other operations should be
permitted.

The code works, I think... except (for example) a = b + 10;
needs to be a = b + (Angle) 10;

[snip]

The standard work around for that is to make the + operator a friend rather
than a member function. Friends are treated better than relatives. Go
figure.

[Michael Mellor]

Darren Grant wrote:

Hi there,

I've attempted to implement an Angle class. An Angle is a subset of an
integer, where the range is [0,360). All other operations should be
permitted.

The code works, I think... except (for example) a = b + 10;
needs to be a = b + (Angle) 10; This is because you have a conversion to int operator. Do you need this
conversion operator? I find that these kind of conversions to basic
types just get in the way. Normally providing a conversion constructor
and the appropriate operator overloads works fine.

Could some kind soul comment on my code and show me how it could be
done better?
#include <iostream>

class Angle {
friend std::ostream& operator<<(std::ostream&, const Angle&) {
os << s.theta;
return os;
} It does help to post the actual code :).
friend std::ostream& operator<<(std::ostream& os, const Angle&s) {

I prefer to avoid friends and you could by doing:
std::ostream& operator<<(std::ostream& os, const Angle&s) {
os << (int)s;
return os;
}
although a method to access to angle could be useful, e.g get_angle().

public:
Angle() : theta(0) { };
This allows theta to be out of range: Angle(int t) : theta(t) { }; If you had a function:
int clamp_angle ( int angle ) {
if ( angle >= 0 )
return angle % 360;
else
return 360 - (angle % 360);
}
then you could put:
Angle(int t) : theta(clamp_angle(t)) { };
Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360; If angles are always in the range [0,360) why take the modulo here?
return *this;
}

Angle& operator+=(const Angle& a) {
theta = (theta + a) % 360;
if (theta < 0)
theta = 360 - abs(theta);
return *this;
} Once again if angles are always positive how can the result be less that 0?
Angle& operator+=(const Angle& a) {
theta = (theta + a) % 360;
return *this;
}
Angle& operator-=(const Angle& a) {
Angle t = a % 360;
theta = theta - t;
if (theta < 0)
theta = 360 - abs(theta);
return *this;
} You could do it more efficiently like this:
Angle& operator-=(const Angle& a) {
theta -= a;
if (theta < 0)
theta += 360;
return *this;
}

operator int() const {
return theta;
} I am not a big fan of these operators.

private:
int theta;
};

Angle operator-(const Angle& l, const Angle& r) {
Angle a = l;
a -= r;
return a;
}

Angle operator+(const Angle& l, const Angle& r) {
Angle a = l;
a += r;
return a;
}

Many thanks,
Darren

[Darren Grant]

On Mon, 15 Dec 2003 16:57:04 +0000, Michael Mellor <news-at-@michaelmellor-
dot-.com> wrote:

Darren Grant wrote:

Hi there,

I've attempted to implement an Angle class. An Angle is a subset of an
integer, where the range is [0,360). All other operations should be
permitted.
Hi there,

Thanks for your time. I've looked over your suggestions, and implemented
some of them, which has made my code a lot better, but there is still one
small
problem. First may I present my updated code;

#include <iostream>

class Angle {
public:
Angle() : theta(0) { };
Angle(int t) : theta(clamp_angle(t)) { };

Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360;
return *this;
}

Angle& operator+=(const Angle& a) {
theta = (theta + a) % 360;
return *this;
}

Angle& operator-=(const Angle& a) {
theta = (theta - a) % 360;
if (theta < 0)
theta += 360;
return *this;
}

operator int() const {
return theta;
}

private:
int theta;

int clamp_angle(int angle) {
if (angle >= 0)
return angle % 360;
else
return 360 + (angle % 360);
}
};

Angle operator-(const Angle&, const Angle&);
Angle operator+(const Angle&, const Angle&);

std::ostream& operator<<(std::ostream& os, const Angle& s) {
os << (int) s;
return os;
}

Angle operator-(const Angle& l, const Angle& r) {
Angle a = l;
a -= r;
return a;
}

Angle operator+(const Angle& l, const Angle& r) {
Angle a = l;
a += r;
return a;
}

I've removed some of the quoted text, and responded to the parts which
are still relevent:
The code works, I think... except (for example) a = b + 10;
needs to be a = b + (Angle) 10; This is because you have a conversion to int operator. Do you need this
conversion operator? I find that these kind of conversions to basic types
just get in the way. Normally providing a conversion constructor and the
appropriate operator overloads works fine.

I don't know how else to do it. If I comment out the conversion, I get
the error 'initializing' : cannot convert from 'const class Angle' to
'int'
for Angle(const Angle& a) : theta(a) { };
(and more errors).

I understand why - I just don't know how else to do it.

class Angle {
friend std::ostream& operator<<(std::ostream&, const Angle&) {
os << s.theta;
return os;
}

It does help to post the actual code :).
friend std::ostream& operator<<(std::ostream& os, const Angle&s) {

Sorry :) More care taken this time.
I prefer to avoid friends and you could by doing:
std::ostream& operator<<(std::ostream& os, const Angle&s) {
os << (int)s;
return os;
}
This works, which is cool. I'm learning C++ from Accelerated C++, which
used a friend in their example.
although a method to access to angle could be useful, e.g get_angle().
I havn't put this in, yet... I was hoping to get away with the int
conversion function.

public:
Angle() : theta(0) { };

This allows theta to be out of range:

Angle(int t) : theta(t) { };

If you had a function:
int clamp_angle ( int angle ) {
if ( angle >= 0 )
return angle % 360;
else
return 360 - (angle % 360);
}
then you could put:
Angle(int t) : theta(clamp_angle(t)) { };

Implemented, although I think you may have made a small mistake with
the sign in the last line of the function; corrected in my code above.

Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360;

If angles are always in the range [0,360) why take the modulo here?

Ah, I meant the description, above, of the angles being in the
range [0,360) as a constraint, not a pre-condition.

Angle& operator-=(const Angle& a) {
Angle t = a % 360;
theta = theta - t;
if (theta < 0)
theta = 360 - abs(theta);
return *this;
}

You could do it more efficiently like this:
Angle& operator-=(const Angle& a) {
theta -= a;
if (theta < 0)
theta += 360;
return *this;
}

I don't think this worked right away, but I did implement a nicer bit of
code based on it.

operator int() const {
return theta;
}

I am not a big fan of these operators.

So, the class works (I have included the code I used to test it with below)
,
and it's a lot nicer than my first attempt, thanks to your email, and an
early
morning start :) But I still have the small problem, where
b + 10; needs to be
b + (Angle) 10;
else I get the error '+' : 2 overloads have similar conversions.

Can you provide any more help to sort this out?

Also,
On Mon, 15 Dec 2003 10:44:00 -0800, osmium <r1********@comcast.net> wrote: The standard work around for that is to make the + operator a friend
rather
than a member function. Friends are treated better than relatives. Go
figure.

I tried this, just by putting
friend Angle operator+(const Angle&, const Angle&);
in the class definition, instead of outside without 'friend', but I get
the error '+' : 2 overloads have similar conversions
for theta = (theta + a) % 360; (in Angle& operator+=()).

So can you show me the correct path? :)

Many thanks for your help,
Darren
Here is the code I have used to test my class with;

using namespace std;

int main(void) {
Angle a(10);
cout << "Angle a(10); a = " << a << endl;

Angle b(a);
cout << "Angle b(a); b = " << b << endl;

a = 50;
cout << "a = 50; a = " << a << endl;

a = b;
cout << "a = b; a = " << a << endl;

Angle c = -15;
cout << "Angle c = -15; c = " << c << endl << endl;

a = 10 + 5;
cout << "small addition; a = 10 + 5;\t\ta = " << a << endl;

a = 355 + 10;
cout << "large addition; a = 355 + 10;\t\ta = " << a << endl;

a = 355 + 710;
cout << "very large addition; a = 355 + 710;\ta = " << a << endl;

a = 10 + -15;
cout << "addition of a negative; a = 10 + -15;\ta = " << a << endl
<< endl;

a = 10 - 5;
cout << "a = 10 - 5; a = " << a << endl;

a = 5 - 10;
cout << "a = 5 - 10; a = " << a << endl;

a = -5 - 10;
cout << "a = -5 - 10; a = " << a << endl;

a = -5 - 370;
cout << "a = -5 - 370; a = " << a << endl << endl;

a = 10;
a += 365;
cout << "a = 10; a += 365;\ta = " << a << endl;

a = 10;
a -= 15;
cout << "a = 10; a -= 15;\ta = " << a << endl;

a = 10;
a -= 355;
cout << "a = 10; a -= 355;\ta = " << a << endl;

a = 10;
a -= 365;
cout << "a = 10; a -= 365;\ta = " << a << endl;

a = 10;
a -= 730;
cout << "a = 10; a -= 730;\ta = " << a << endl << endl;

cout << "b + 10 = " << (b + (Angle) 10) << endl;
cout << "b - 30 = " << (b - (Angle) 30) << endl;

return 0;
}

[Michael Mellor]

Darren Grant wrote:

On Mon, 15 Dec 2003 16:57:04 +0000, Michael Mellor
<news-at-@michaelmellor- dot-.com> wrote:

Darren Grant wrote:

Hi there,

I've attempted to implement an Angle class. An Angle is a subset of an
integer, where the range is [0,360). All other operations should be
permitted. [...]

This allows theta to be out of range:

Angle(int t) : theta(t) { };
If you had a function:
int clamp_angle ( int angle ) {
if ( angle >= 0 )
return angle % 360;
else
return 360 - (angle % 360);
}
then you could put:
Angle(int t) : theta(clamp_angle(t)) { };

Implemented, although I think you may have made a small mistake with
the sign in the last line of the function; corrected in my code above.

I am guilty of never testing the code.
Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360;

If angles are always in the range [0,360) why take the modulo here?

Ah, I meant the description, above, of the angles being in the
range [0,360) as a constraint, not a pre-condition.

I am confused by the difference between constraint and pre-condition. As
I currently see it is your constraint is that theta will be in the range
[0,360) which implies the pre-condition for all methods of theta being
in that range.

[...]
I tried this, just by putting
friend Angle operator+(const Angle&, const Angle&);
in the class definition, instead of outside without 'friend', but I get
the error '+' : 2 overloads have similar conversions
for theta = (theta + a) % 360; (in Angle& operator+=()).

So can you show me the correct path? :)

You would have to remove the conversion operator and implement the class
something like this:
class Angle {
public:
Angle() : theta(0) { };
Angle(int t) : theta(clamp_angle(t)) { };

Angle(const Angle& a) : theta(a.get_angle()) { };

Angle& operator=(const Angle& rhs) {
// This modulo really isn't needed, if you allow an angle to
// be out of range why not allow the one your assigning it
// to be out of range?
theta = rhs.get_angle() % 360;
return *this;
}

Angle& operator+=(const Angle& a) {
theta = (theta + a.get_angle()) % 360;
return *this;
}

Angle& operator-=(const Angle& a) {
// This modulo can also go if you keep theta in range
// which the class does do
theta = (theta - a.get_angle()) % 360;
if (theta < 0)
theta += 360;
return *this;
}

int get_angle () const {
return theta;
}
private:
int theta;

int clamp_angle(int angle) {
if (angle >= 0)
return angle % 360;
else
return 360 + (angle % 360);
}
};

then this would work
Angle a(10);
Angle b(a);
cout << "b + 10 = " << (b + 10) << '\n';
cout << "b - 30 = " << (b - 30) << '\n';

BTW endl terminates the line and flushes the buffer, '\n' or "\n" just
terminate the line.

Mike

[Darren Grant]

On Tue, 16 Dec 2003 14:24:05 +0000, Michael Mellor <news-at-@michaelmellor-
dot-.com> wrote:

[...]

Angle(const Angle& a) : theta(a) { };

Angle& operator=(const Angle& rhs) {
theta = rhs % 360;

If angles are always in the range [0,360) why take the modulo here?
Ah, I meant the description, above, of the angles being in the
range [0,360) as a constraint, not a pre-condition.

I am confused by the difference between constraint and pre-condition. As
I currently see it is your constraint is that theta will be in the range
[0,360) which implies the pre-condition for all methods of theta being in
that range.

Ok, I'll drop the jargon I don't fully understand. I simply meant that,
although any value (eg -400) can be assigned to an Angle, they will always
be clamped to [0,360). So,
a = -10; // a == 350
a = 370; // a == 10
[...]

I tried this, just by putting
friend Angle operator+(const Angle&, const Angle&);
in the class definition, instead of outside without 'friend', but I get
the error '+' : 2 overloads have similar conversions
for theta = (theta + a) % 360; (in Angle& operator+=()).

So can you show me the correct path? :)
You would have to remove the conversion operator and implement the class
something like this:
class Angle {
public:
Angle() : theta(0) { };
Angle(int t) : theta(clamp_angle(t)) { };

Angle(const Angle& a) : theta(a.get_angle()) { };

Cool, this works.
Angle& operator=(const Angle& rhs) {
// This modulo really isn't needed, if you allow an angle to
// be out of range why not allow the one your assigning it
// to be out of range?
theta = rhs.get_angle() % 360;
return *this;
}
I don't allow an angle to be out of range (or rather, if an angle is
assigned an out of range value, it is clamped), but I found that I
could remove this operator anyway - and the Angle(int t) constructor
is of course used instead.

I also rewrote the += operator;
Angle& operator+=(const Angle& a) {
*this = theta + a.get_angle();
return *this;
}
I hope this is good technique?
BTW endl terminates the line and flushes the buffer, '\n' or "\n" just
terminate the line.

Oh, ta.

Many thanks for your help - I really understand this a lot better now.

My code now looks like this - smaller and simpler than before:

class Angle {

friend Angle operator-(const Angle& l, const Angle& r);
friend Angle operator+(const Angle& l, const Angle& r);

public:
Angle() : theta(0) { };
Angle(int t) : theta(clamp_angle(t)) { };

Angle(const Angle& a) : theta(a.get_angle()) { };

Angle& operator+=(const Angle& a) {
*this = theta + a.get_angle();
return *this;
}

Angle& operator-=(const Angle& a) {
*this = theta - a.get_angle();
return *this;
}

int get_angle() const {
return theta;
}

private:
int theta;

int clamp_angle(int angle) {
if (angle >= 0)
return angle % 360;
else
return 360 + (angle % 360);
}
};

std::ostream& operator<<(std::ostream& os, const Angle& s) {
os << s.get_angle();
return os;
}

Angle operator-(const Angle& l, const Angle& r) {
Angle a = l;
a -= r;
return a;
}

Angle operator+(const Angle& l, const Angle& r) {
Angle a = l;
a += r;
return a;
}
Thanks again,
Darren