Hi there,
I'm learning C++, and just wrote a 2D point class. Would some kind soul
be able to look over my code and give me any constructive criticism they
can think of?
#ifndef POINT_H
#define POINT_H
#include <iostream>
class Point {
friend bool operator==(Point&, Point&);
friend bool operator!=(Point&, Point&);
friend Point operator-(const Point& l, const Point& r);
friend Point operator+(const Point& l, const Point& r);
friend Point operator*(const Point& l, double r);
friend Point operator/(const Point& l, double r);
public:
Point() : _x(0), _y(0) { };
Point(double x, double y) : _x(x), _y(y) { };
void X(double x) { _x = x; } double X() const { return _x; }
void Y(double y) { _y = y; } double Y() const { return _y; }
Point& operator+=(const Point& p);
Point& operator-=(const Point& p);
double dot(const Point&) const;
double norm(void) const;
Point normal(void) const;
Point proj(const Point&) const;
private:
double _x, _y;
};
std::ostream& operator<<(std::ostream&, const Point&);
#endif
#include <math.h>
#include "Point.h"
std::ostream& operator<<(std::ostream& os, const Point& s) {
os << "(" << s.X() << "," << s.Y() << ")";
return os;
}
bool operator==(Point& p1, Point& p2) {
return (p1._x == p2._x && p1._y == p2._y);
}
bool operator!=(Point& p1, Point& p2) {
return (p1._x != p2._x || p1._y != p2._y);
}
Point operator+(const Point& l, const Point& r) {
Point p = l;
p += r;
return p;
}
Point operator-(const Point& l, const Point& r) {
Point p = l;
p -= r;
return p;
}
Point operator*(const Point &l, double r) {
Point p = l;
p._x *= r;
p._y *= r;
return p;
}
Point operator/(const Point &l, double r) {
Point p = l;
p._x /= r;
p._y /= r;
return p;
}
Point& Point::operator+=(const Point& p) {
_x += p._x;
_y += p._y;
return *this;
}
Point& Point::operator-=(const Point& p) {
_x -= p._x;
_y -= p._y;
return *this;
}
double Point::dot(const Point& op) const {
return _x * op.X() + _y * op.Y();
}
double Point::norm(void) const {
return sqrt(_x * _x + _y * _y);
}
Point Point::normal(void) const {
return *this / norm();
}
Point Point::proj(const Point& u) const {
return (*this * u.dot(*this)) / (norm() * norm());
}
Many thanks,
Darren Grant 8 1200
Darren Grant wrote: Hi there,
I'm learning C++, and just wrote a 2D point class. Would some kind soul be able to look over my code and give me any constructive criticism they can think of?
#ifndef POINT_H #define POINT_H
#include <iostream>
class Point { friend bool operator==(Point&, Point&); friend bool operator!=(Point&, Point&); friend Point operator-(const Point& l, const Point& r); friend Point operator+(const Point& l, const Point& r); friend Point operator*(const Point& l, double r); friend Point operator/(const Point& l, double r);
public: Point() : _x(0), _y(0) { }; Point(double x, double y) : _x(x), _y(y) { };
void X(double x) { _x = x; } double X() const { return _x; } void Y(double y) { _y = y; } double Y() const { return _y; }
Point& operator+=(const Point& p); Point& operator-=(const Point& p);
double dot(const Point&) const; double norm(void) const; Point normal(void) const; Point proj(const Point&) const;
private: double _x, _y; };
std::ostream& operator<<(std::ostream&, const Point&);
#endif #include <math.h>
#include "Point.h"
std::ostream& operator<<(std::ostream& os, const Point& s) { os << "(" << s.X() << "," << s.Y() << ")"; return os; }
bool operator==(Point& p1, Point& p2) { return (p1._x == p2._x && p1._y == p2._y); }
bool operator!=(Point& p1, Point& p2) { return (p1._x != p2._x || p1._y != p2._y); }
Point operator+(const Point& l, const Point& r) { Point p = l; p += r; return p; }
Point operator-(const Point& l, const Point& r) { Point p = l; p -= r; return p; }
Point operator*(const Point &l, double r) { Point p = l; p._x *= r; p._y *= r; return p; }
Point operator/(const Point &l, double r) { Point p = l; p._x /= r; p._y /= r; return p; }
Point& Point::operator+=(const Point& p) { _x += p._x; _y += p._y; return *this; }
Point& Point::operator-=(const Point& p) { _x -= p._x; _y -= p._y; return *this; }
double Point::dot(const Point& op) const { return _x * op.X() + _y * op.Y(); }
double Point::norm(void) const { return sqrt(_x * _x + _y * _y); }
Point Point::normal(void) const { return *this / norm(); }
Point Point::proj(const Point& u) const { return (*this * u.dot(*this)) / (norm() * norm()); }
Many thanks, Darren Grant
since you overloaded * and / for scalars, for semantic completeness, you should probably overload *= and /= as well.
Point& Point::operator*=(double d)
{
_x *= d;
_y *= d;
return *this;
}
Point& Point::operator/=(double d)
{
_x /= d;
_y /= d;
return *this;
}
"Darren Grant" <dg**@hotmail.com> wrote in message
news:op**************@news.hotmail.com... Hi there,
I'm learning C++, and just wrote a 2D point class. Would some kind soul be able to look over my code and give me any constructive criticism they can think of?
#ifndef POINT_H #define POINT_H
#include <iosfwd> instead 'cos you don't actually use ostream in the header
#include <iostream>
class Point {
These don't need to be friends as the X() and Y() methods are inlined and
will do the job perfectly well
friend bool operator==(Point&, Point&); friend bool operator!=(Point&, Point&);
The difference between two points is not a point
friend Point operator-(const Point& l, const Point& r);
It is not meaningful to add two points
friend Point operator+(const Point& l, const Point& r);
Obviously this is intended to be scaling but even here you should implement
*= and /= as members and implement * and /
as non-friend free functions in terms of these.
friend Point operator*(const Point& l, double r);
define behaviour for divide by 0
friend Point operator/(const Point& l, double r);
public: Point() : _x(0), _y(0) { }; Point(double x, double y) : _x(x), _y(y) { };
I would prefer not to have manipulators for X and Y - It is cleaner to just
use constructors as in
X a;
a = Point(a.X(),42);
For small classes with simple ctors I usually prefer no mutating methods at
all.
Use of uppercase for methods is inconsistent and contrary to most coding
standards.
void X(double x) { _x = x; } double X() const { return _x; } void Y(double y) { _y = y; } double Y() const { return _y; }
Point& operator+=(const Point& p); Point& operator-=(const Point& p);
double dot(const Point&) const;
(void) is only useful for C compatibility and since a class member cannot be
C compatible it has no place here.
double norm(void) const; Point normal(void) const; Point proj(const Point&) const;
private:
I'm not sure on the exact details of when you can or cannot use leading _
and nor are most other C++ programmers.
us x_,y_ or m_x,m_y and you'll be certain that it wont clash with
implementation stuff.
double _x, _y; };
std::ostream& operator<<(std::ostream&, const Point&);
#endif #include <math.h>
#include "Point.h"
#include <iostream> std::ostream& operator<<(std::ostream& os, const Point& s) { os << "(" << s.X() << "," << s.Y() << ")"; return os; }
bool operator==(Point& p1, Point& p2) { return (p1._x == p2._x && p1._y == p2._y); }
bool operator!=(Point& p1, Point& p2) { return (p1._x != p2._x || p1._y != p2._y); }
The siimplest and potentially most efficient way to do the following sort
of thing is -
Point operator+(Point l,const Point&r)
{
return l+=r;
}
Point operator+(const Point& l, const Point& r) { Point p = l; p += r; return p; }
Point operator-(const Point& l, const Point& r) { Point p = l; p -= r; return p; }
Point operator*(const Point &l, double r) { Point p = l; p._x *= r; p._y *= r; return p; }
Point operator/(const Point &l, double r) { Point p = l; p._x /= r; p._y /= r; return p; }
Point& Point::operator+=(const Point& p) { _x += p._x; _y += p._y; return *this; }
Point& Point::operator-=(const Point& p) { _x -= p._x; _y -= p._y; return *this; }
double Point::dot(const Point& op) const { return _x * op.X() + _y * op.Y(); }
double Point::norm(void) const { return sqrt(_x * _x + _y * _y); }
Point Point::normal(void) const { return *this / norm(); }
Point Point::proj(const Point& u) const { return (*this * u.dot(*this)) / (norm() * norm()); }
Many thanks, Darren Grant
Only thing I would say is use '#include <cmath>' to be more standards
compliant.
Kamuela Franco
On Wed, 24 Dec 2003 21:42:27 -0000, Nicholas Hounsome
<nh********@blueyonder.co.uk> wrote: "Darren Grant" <dg**@hotmail.com> wrote in message news:op**************@news.hotmail.com... Hi there,
I'm learning C++, and just wrote a 2D point class. Would some kind soul be able to look over my code and give me any constructive criticism they can think of?
#ifndef POINT_H #define POINT_H
#include <iosfwd> instead 'cos you don't actually use ostream in the header
Done. #include <iostream>
class Point { These don't need to be friends as the X() and Y() methods are inlined and will do the job perfectly well
friend bool operator==(Point&, Point&); friend bool operator!=(Point&, Point&);
Ok, avoid friendship where you don't need, fair enough.
The difference between two points is not a point
friend Point operator-(const Point& l, const Point& r); It is not meaningful to add two points
friend Point operator+(const Point& l, const Point& r);
Obviously this is intended to be scaling but even here you should
Oh? "To add two (or more) vectors together, all you have to do is add the
respective components together." "u + v = <u_x + v_x, u_y + v_y>"
(from Tricks of the 3D Game Programming Gurus by Andre Lamothe).
Similarly for subtraction.
Scaling is done with multiplication by a scalar:
implement *= and /= as members and implement * and / as non-friend free functions in terms of these.
friend Point operator*(const Point& l, double r);
Done.
define behaviour for divide by 0
if (r == 0)
throw std::domain_error("divide by zero");
friend Point operator/(const Point& l, double r);
public: Point() : _x(0), _y(0) { }; Point(double x, double y) : _x(x), _y(y) { };
I would prefer not to have manipulators for X and Y - It is cleaner to just use constructors as in X a; a = Point(a.X(),42); For small classes with simple ctors I usually prefer no mutating methods at all.
I guess this is personal preference. I got this idea from Design Patterns
(http://www.amazon.com/exec/obidos/tg...2/102-9465371-
5694567?v=glance)
Use of uppercase for methods is inconsistent and contrary to most coding standards.
void X(double x) { _x = x; } double X() const { return _x; } void Y(double y) { _y = y; } double Y() const { return _y; }
Ok, done.
Point& operator+=(const Point& p); Point& operator-=(const Point& p);
double dot(const Point&) const; (void) is only useful for C compatibility and since a class member cannot be C compatible it has no place here.
Oh, ok. double norm(void) const; Point normal(void) const; Point proj(const Point&) const;
private:
I'm not sure on the exact details of when you can or cannot use leading _ and nor are most other C++ programmers. us x_,y_ or m_x,m_y and you'll be certain that it wont clash with implementation stuff.
Ok, changed. double _x, _y; };
std::ostream& operator<<(std::ostream&, const Point&);
#endif #include <math.h>
#include "Point.h"
#include <iostream>
std::ostream& operator<<(std::ostream& os, const Point& s) { os << "(" << s.X() << "," << s.Y() << ")"; return os; }
bool operator==(Point& p1, Point& p2) { return (p1._x == p2._x && p1._y == p2._y); }
bool operator!=(Point& p1, Point& p2) { return (p1._x != p2._x || p1._y != p2._y); }
The siimplest and potentially most efficient way to do the following sort of thing is -
Point operator+(Point l,const Point&r) { return l+=r; }
But then l is modified. Eg
m / 2; // m is changed Point operator+(const Point& l, const Point& r) { Point p = l; p += r; return p; }
Point operator-(const Point& l, const Point& r) { Point p = l; p -= r; return p; }
Point operator*(const Point &l, double r) { Point p = l; p._x *= r; p._y *= r; return p; }
Point operator/(const Point &l, double r) { Point p = l; p._x /= r; p._y /= r; return p; }
Point& Point::operator+=(const Point& p) { _x += p._x; _y += p._y; return *this; }
Point& Point::operator-=(const Point& p) { _x -= p._x; _y -= p._y; return *this; }
double Point::dot(const Point& op) const { return _x * op.X() + _y * op.Y(); }
double Point::norm(void) const { return sqrt(_x * _x + _y * _y); }
Point Point::normal(void) const { return *this / norm(); }
Point Point::proj(const Point& u) const { return (*this * u.dot(*this)) / (norm() * norm()); }
Cool, thanks very much for your help. I definately learned a lot.
Regards,
Darren
"Darren Grant" <dg**@hotmail.com> wrote in message
news:op**************@news.hotmail.com... On Wed, 24 Dec 2003 21:42:27 -0000, Nicholas Hounsome <nh********@blueyonder.co.uk> wrote:
[snip] Oh? "To add two (or more) vectors together, all you have to do is add the respective components together." "u + v = <u_x + v_x, u_y + v_y>" (from Tricks of the 3D Game Programming Gurus by Andre Lamothe).
Similarly for subtraction.
Scaling is done with multiplication by a scalar:
True but the class is called Point not Vector therefor I assume that is
supposed to represent a point and not a vector hence....
The naming of things is probably the most important and underrated part of
OO design -
consider bery carefully what a class is and have it do only appropriate
methods for that abstraction and users/maintainers will be able to get
around your code using their natural intuition and doamin knowledge.
[snip] I would prefer not to have manipulators for X and Y - It is cleaner to just use constructors as in X a; a = Point(a.X(),42); For small classes with simple ctors I usually prefer no mutating
methods at all.
I guess this is personal preference. I got this idea from Design Patterns (http://www.amazon.com/exec/obidos/tg...2/102-9465371- 5694567?v=glance)
Which pattern?
You may find that immutable types are desirable in multithreaded
applications also - following on from the previous comment - the abstraction
is a point not a pair of x,y coordinates - it's not an absolute - more a
question of emphasis - you may have read several books going on about how
you might want to use polar coordinates as the internal representation (No
I've never found a use for it either).
[snip] The siimplest and potentially most efficient way to do the following sort of thing is -
Point operator+(Point l,const Point&r) { return l+=r; }
But then l is modified. Eg
m / 2; // m is changed
No it isn't because l is not passed by reference so it is a copy of m i.e.
the compiler has automatically done what you did by writing
Point temp = l;
Darren Grant wrote: It is not meaningful to add two points friend Point operator+(const Point& l, const Point& r); Oh? "To add two (or more) vectors together, all you have to do is add the respective components together." "u + v = <u_x + v_x, u_y + v_y>"
Yes, but points are not vectors. If you want a vector, call the class
'Vector'! Adding two points does not make sense, however adding a vector to
a point does! Having two classes, 'Point' (P) and 'Vector' (V), the
following +/- operations should be supported:
V + V = V
V - V = V
P + V = P
P - P = V
Everything else (like 'P + P') is undefined!
cu
Heiko
On Fri, 26 Dec 2003 08:30:38 -0000, Nicholas Hounsome" nospace Hounsome
<@blueyonder.co.uk> wrote: "Darren Grant" <dg**@hotmail.com> wrote in message news:op**************@news.hotmail.com... On Wed, 24 Dec 2003 21:42:27 -0000, Nicholas Hounsome <nh********@blueyonder.co.uk> wrote: [snip] Oh? "To add two (or more) vectors together, all you have to do is add the respective components together." "u + v = <u_x + v_x, u_y + v_y>" (from Tricks of the 3D Game Programming Gurus by Andre Lamothe).
Similarly for subtraction.
Scaling is done with multiplication by a scalar:
True but the class is called Point not Vector therefor I assume that is supposed to represent a point and not a vector hence....
True, I didn't call it Vector because of the STL vector, and also it's
not as general as an n-dimensional vector - it's only a 2D vector.
But... what's the difference between a 2D point and a 2D vector? My reason
for calling the class Point is because I was under the impression there is
none...
The naming of things is probably the most important and underrated part of OO design - consider bery carefully what a class is and have it do only appropriate methods for that abstraction and users/maintainers will be able to get around your code using their natural intuition and doamin knowledge.
[snip]
> > I would prefer not to have manipulators for X and Y - It is cleaner to > just > use constructors as in > X a; > a = Point(a.X(),42); > For small classes with simple ctors I usually prefer no mutating methods > at all. I guess this is personal preference. I got this idea from Design Patterns (http://www.amazon.com/exec/obidos/tg...2/102-9465371- 5694567?v=glance)
Which pattern?
Right at the end, in 'Foundation classes'. Only the class definition
is given, not the implementation.
You may find that immutable types are desirable in multithreaded applications also - following on from the previous comment - the abstraction is a point not a pair of x,y coordinates - it's not an absolute - more a question of emphasis - you may have read several books going on about how you might want to use polar coordinates as the internal representation (No I've never found a use for it either). [snip]
> The siimplest and potentially most efficient way to do the following > sort > of thing is - > > Point operator+(Point l,const Point&r) > { > return l+=r; > }
But then l is modified. Eg
m / 2; // m is changed
No it isn't because l is not passed by reference so it is a copy of m i.e. the compiler has automatically done what you did by writing Point temp = l;
Damn. I left the & in. :)
Cheers,
Darren
"Darren Grant" wrote: True but the class is called Point not Vector therefor I assume that is supposed to represent a point and not a vector hence....
True, I didn't call it Vector because of the STL vector, and also it's not as general as an n-dimensional vector - it's only a 2D vector.
But... what's the difference between a 2D point and a 2D vector? My reason for calling the class Point is because I was under the impression there is none...
Most graphics textbooks have introductory chapters that describe the
differences between points and vectors, which are not the same as far
as mathematical operations are concerned. For example, P+V=P, P-P=V,
P+P is not strictly legal, etc. If you want to go further, there is a
third class, normals, which are vectors that are treated differently
in certain respects. (Incidentally, the existence of normals makes
your normal() member function a potential source of confusion; perhaps
normalize() would be a better name.)
As Nicholas pointed out, naming is very important. You could call
your class Vec2d or Vector2d to more precisely describe its meaning,
and to eliminate any confusion with std::vector. Or put it in a
namespace, like Math2d, so Math2d::Vector will be its fully-qualified
name. This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics
by: Mudge |
last post by:
Please, someone, tell me why OO in PHP is better than procedural.
|
by: bigoxygen |
last post by:
Hi.
I'm using a 3 tier FrontController Design for my web application right
now.
The problem is that I'm finding to have to duplicate a lot of code for
similar functions; for example, listing...
|
by: jason |
last post by:
Hello.
Newbie on SQL and suffering through this.
I have two tables created as such:
drop table table1;
go
drop table table2;
go
|
by: Dario de Judicibus |
last post by:
I'm getting crazy. Look at this code:
#include <string.h>
#include <stdio.h>
#include <iostream.h>
using namespace std ;
char ini_code = {0xFF, 0xFE} ;
char line_sep = {0x20, 0x28} ;
|
by: Andrew Thompson |
last post by:
It seems most people get there JS off web sites, which is
entirely logical. But it is also a great pity since most
of that code is of such poor quality.
I was looking through the JS FAQ for any...
|
by: ED |
last post by:
I currently have vba code that ranks employees based on their average
job time ordered by their region, zone, and job code. I currently
have vba code that will cycle through a query and ranks each...
|
by: Namratha Shah \(Nasha\) |
last post by:
Hey Guys,
Today we are going to look at Code Access Security.
Code access security is a feature of .NET that manages code depending on its
trust level. If the CLS trusts the code enough to...
|
by: Joe Fallon |
last post by:
I have some complex logic which is fairly simply to build up into a string.
I needed a way to Eval this string and return a Boolean result.
This code works fine to achieve that goal.
My...
|
by: Alan Silver |
last post by:
Hello,
Newbie here, so please forgive what is probably a basic question ...
I see a lot of discussion about "code behind", which if I have
understood correctly, means that the script code goes...
|
by: tshad |
last post by:
I am just trying to decide whether to split my code and uses code behind. I
did it with one of my pages and found it was quite a bit of trouble.
I know that most people (and books and articles)...
|
by: DolphinDB |
last post by:
Tired of spending countless mintues downsampling your data? Look no further!
In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
|
by: isladogs |
last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM).
In this month's session, we are pleased to welcome back...
|
by: isladogs |
last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM).
In this month's session, we are pleased to welcome back...
|
by: jfyes |
last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
|
by: ArrayDB |
last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
|
by: CloudSolutions |
last post by:
Introduction:
For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
|
by: Defcon1945 |
last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work
|
by: Shællîpôpï 09 |
last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
|
by: af34tf |
last post by:
Hi Guys, I have a domain whose name is BytesLimited.com, and I want to sell it. Does anyone know about platforms that allow me to list my domain in auction for free. Thank you
| |