I've been handed this assignment where I should make two classes, one Polygon class and one Point class.

The polygon class consists of Points and I'm trying to overload Polygon operator +(Point point); so that the + operator adds the values of the points x and y to all the points in the Polygon.

The same goes for the operator * I'm trying to overload it so that it multiplies all the x and y's of the Polygon with the points x and y values..

It works to some extent but can't really see where I've failed. I believe it's in the copy constructor..

If it would work as intended the output of this program would be:
6,5
7,6
8,7

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147`` ``````#include using namespace std; class Point { public: Point(float _x=1, float _y=1) { x = _x, y=_y; } Point operator + (Point pt) { float _x = x + pt.x; float _y = y + pt.y; return Point(_x,_y); } Point operator - (Point pt) { float _x = x - pt.x; float _y = y - pt.y; return Point(_x,_y); } Point operator * (Point pt) { float _x = (x * pt.x) - (y * pt.y); float _y = (x * pt.y) + (y * pt.x); return Point(_x,_y); } friend ostream& operator << (ostream& os,Point pt); private: float x; float y; }; class Polygon { public: Polygon(int i = 0) { _point = new Point[i]; index = i; } Polygon operator + (Polygon polygon) { } Polygon operator + (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { polygon[i] = polygon[i] + point; } return polygon; } Point& operator [](int i) { return _point[i]; } Polygon operator * (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { polygon[i] = polygon[i] * point; } return polygon; } Polygon& operator =(Polygon polygon) { delete[] _point; index = polygon.index; if(polygon._point) { _point = new Point[index]; *_point = *polygon._point; } else { _point = 0; return *this; } } Polygon(const Polygon &polygon) // Copy constructor { index = polygon.index; for(int i = 0; i < index; i++) { if(polygon._point) { _point = new Point[index]; *_point = *polygon._point; } else _point = 0; } } ~Polygon() { delete [] _point; } friend ostream& operator << (ostream& os, Polygon polygon); private: Point *_point; int index; }; ostream& operator<<(ostream& os, Point point) { os << point.x << "," << point.y; return os; } ostream& operator <<(ostream& os, Polygon polygon) { for(int i = 0; i < polygon.index ;i++) { os << polygon[i] << endl; } return os; } int main() { Polygon polygon1(3); polygon1[0] = Point(0,0); polygon1[1] = Point(1,1); polygon1[2] = Point(2,2); Point point1(6,5); polygon1 = polygon1 + point1; cout << polygon1 << endl; system("pause"); return 0; }``````
Last edited on
I would recommend changing the Polygon::_point from "Point *" to "std::vector<Point>". Then update member functions as necessary.
I've been told not to use vectors since we haven't gotten to that in the course yet..
The copy assignment operator of class poligon is invalid. First of all you does not check whether the object is assigned to himself. and the following line in the operator

*_point = *polygon._point;

assignes only the first element of the array _point. All other elements have undefined values.
Last edited on

Hope you don't mind this very long post..
I believe the process of debugging gives a lot more valuable experience than knowing the bug / correct solution.
(hopefully you'll get something more from this post... apart from where you went wrong)

okay, how do we debug your code ? Let's Try testing it incrementally, part by part...
You were doubtful about the copy constructor. So Lets test it first ..
(Refer Note 1)

[Test1]: Testing the Copy Constructor:
 ``1234567891011121314151617181920212223242526`` ``````int main() { Polygon polygon1(3); polygon1[0] = Point(0,0); polygon1[1] = Point(1,1); polygon1[2] = Point(2,2); Point point1(6,5); // Testing copy constructor cout << polygon1[0] << endl; cout << polygon1[1] << endl; cout << polygon1[2] << endl; cout << endl; Polygon p1 = polygon1; // This invokes the copy constructor cout << p1[0] << endl; cout << p1[1] << endl; cout << p1[2] << endl; // polygon1 = polygon1 + point1; // cout << polygon1 << endl; system("pause"); return 0; }``````

(refer Note 2)

output:
 ```0,0 1,1 2,2 0,0 1,1 1,1 ```

This is obviously wrong..
Now, Why is it wrong ?

Problem 1:
 ``1234567891011121314151617`` ``````Polygon(const Polygon &polygon) // Copy constructor { index = polygon.index; for(int i = 0; i < index; i++) // index number of times { if(polygon._point) // if polygon._point is not NULL { _point = new Point[index]; // allocate memory for 'index' Point objects // and store the starting address in _point *_point = *polygon._point; // see below .. } else _point = 0; } } ``````

couple of points regarding the above:
1. you are allocating memory for 'index' Point objects each time you are passing through the loop and you run the loop 'index' times. Totally you are allocating memory for 'index' * 'index' Point objects of which only the last 'index' are accessible and rest are leaked memory.

2. better to use *(polygon._point) so we don't have to worry about precedence

Solution:
I don't understand how you came up with the above code for the copy constructor. So, I couldn't 'correct' it.

When you have dynamic content in an object. In its (i.e. it's class's) copy constructor you want to do the following:
1. copy static members
2. allocate memory for dynamic members
3. copy the dynamic members

Here is my version of the code:
 ``12345678910111213`` `````` Polygon(const Polygon &polygon) // Copy constructor { // copy static members index = polygon.index; // allocate memory for dynamic members _point = new Point[index]; // copy the dynamic members for(int i=0; i

Test this using [Test1] above and the copy constructor should be fine ..

PROBLEM 2:
but there is one more simple problem with your code on line 57
`polygon[i] = polygon[i] + point;`
and on line 70
`polygon[i] = polygon[i] * point;`
try solving it and if you cant post in this thread again .. i'll help you with it :)

[HINT
1. Try thinking about what you want to add/multiply to what and where you want to store the result
2. check if this is what your code does]

---------
Note [1]:
Ideally you'd start with the smallest independent blocks, like the point class functions, etc. that the rest of the program depends on and proceed from there ... but for now, let's start with the constructor
---------

-----------
Note [2]:
I have intentionally avoided using something like ..

`cout << polygon1 << endl;`

because passing an object by value to a function invokes the copy constructor to copy actual parameter into the formal parameter.

For example:
 ``123456789101112`` `````` void func(Polygon obj1) { .... .... } int main() { ... func(poly_obj); // This invokes the copy constructor ... }``````

And if you don't define a copy constructor explicitly, the compiler provides a default copy constructor that copies member by member. Therefore objects with dynamic memory may not copied properly.

case in point:
`ostream& operator <<(ostream& os, Polygon polygon) // this invokes copy constructor `

You can verify this by adding something like ...
`cout << "\tcopy constructor invoked .." << endl;`

Try and verify yourself ... After all, not everything posted on the internet is true :P
-----------
Last edited on
Thanks for the long post, really appreciate the effort.

I changed all the operators to be call by reference instead as well, The reason why I didn't have it was because it worked (to some extent atleast) and I didn't think that it was causing the problem.

I've been trying to figure out the 2nd problem that you posted but can't really see what I'm doing wrong. I want the x and y values of the point to be added to all the points of the polygon. I want to store the values of polygon[i] + point in polygon[i].
I just want the values of x,y of point to be added to all x,y's of polygon[i].

I'm sorry if I sound stupid but why can't I write it like that? To me it makes sense because you can do the same with simple variables like a = a + b.

This is how I would've written it, but it doesn't work since the variables are inaccessible from the class Polygon:

 ``12345`` `````` for(int i = 0; i < index; i++) { polygon[i].x =polygon[i].x + point.x; polygon[i].y = polygon[i].y + pooint.y; }``````

I'm sorry if this is basic stuff for you and it all seems very simple, but I've been reading the chapters that I've been pointed to in my book (Absolute C++, 5th edition )regarding this assignment about overloading operators, memory management and what not. I've read the lecture material that I've been given multiple times but it just doesn't go through this stuff thoroughly. In the book for example they just go on about how to make a constructor and a destructor pretty much, I've been pointed to the rule of three multiple times by other people and it's not even mentioned in the book regarding those areas...
Last edited on
Hey, there's nothing stupid about it...
Either you know it or you don't. Everyone was a beginner at some point of time :)

Okay, So let's see why your code is wrong (i.e) what exactly happens in your code ...

 ``123456789`` ``````Polygon operator + (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { polygon[i] = polygon[i] + point; } return polygon; }``````

On line 3:
you are creating a new object named 'polygon' of the class 'Polygon'

When a new object is created the corresponding constructor is called and it allocates memory using the new operator. In this case the parameterised constructor of the polygon class, on line 42 from your original code.

Also, when a new operator is used to allocate memory for a class object it calls the corresponding constructor of the class (i.e) the following statement

`_point = new Point[i];`

calls the constructor of the Point class

To sum it up, by the time line 3 is done you have 'index' number of 'Point' objects in the newly created object named 'polygon' and they are all initialized to (1,1)

On Line 6:

From above polygon[i] for i = 0,1,2 is (1,1).
and we know 'point' is (6,5)

So,
`polygon[i] = polygon[i] + point;`

is equivalent to (1,1) + (6,5) which is (7,6)
This is what we would get if we test the code.

Solution:

Firstly, lets talk about Calling objects
when you have something like ...

`obj1.add(obj2)`

obj1 is the calling object and obj2 is the parameter passed to obj1's member function 'add'.
Similarly, when you have

`poly1 + poly2;`

it is equivalent to

 ``123`` ``````poly1.operator+(poly2) // here 'operator+' is the function name // it corresponds to the function we are discussing about ``````

Here, the left operand, poly1, is the calling object. And since it is the calling object you can access its members directly inside the function (i.e) you can access _point of poly1 directly in the 'operator+' function

So the modified operator+ function would be

 ``123456789101112`` `````` Polygon operator + (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { // polygon[i] = polygon[i] + point; // above line is changed to the following line polygon[i] = _point[i] + point; } return polygon; }``````

EDIT:
If you still want some clarification feel free to ask ... :)

Ya, it can be tough learning form books .... I've been there too, best way is to try experimenting with small things, learn what happens and ask someone if you are stuck somewhere ... but do not give up. Programming languages by themselves are complex and there is a lot of interdependence between the topics so it's up to the author what he decides to explain first ... but keep trying and soon it'll just clear up and you'll start to see the big picture.
Last edited on
Thank you very much once again, it do makes so much more sense when you point it out to write

`polygon[i] = _point[i] + point;`

It's just stupid to have what I had earlier when you think about it...

I also fixed the error which only copied the first point to the polygon and left all the other ones at their default values..

 ``1234567891011121314151617`` `````` Polygon& operator =(Polygon polygon) { delete[] _point; index = polygon.index; if(polygon._point) { _point = new Point[index]; *_point = *polygon._point; } else { _point = 0; return *this; } }``````

Like why would I even use a IF there if I wanted to copy ALL the points?

This is how it looks now and the program works as intended:

 ``12345678910111213`` `````` Polygon& operator =(const Polygon &polygon) { delete[] _point; index = polygon.index; _point = new Point[index]; for(int i = 0; i< index; i++) { _point[i] = polygon._point[i]; } return *this; }``````

Only thing left is for me to overload Polygon operator + (Polygon polygon); which hopefully won't be so hard.

Ah well thank you very much once again for your kindness, time and effort to help a beginner like myself. :)

Last edited on
This is how I would've written it, but it doesn't work since the variables are inaccessible from the class Polygon:

 ``12345`` `````` for(int i = 0; i < index; i++) { polygon[i].x =polygon[i].x + point.x; polygon[i].y = polygon[i].y + pooint.y; }``````

The interface for the Point class is incomplete - you need public getX & getY functions, so you can retrieve these values.

 ``1234`` `````` for(int i = 0; i < index; i++){ polygon[i].x =polygon[i].x + point.getX; polygon[i].y = polygon[i].y + point.getY; }``````

HTH
That was just an example of how I wanted the values to be added to one another nothing that I was going to write, just showing my way of thinking..
Never the less I think you still need the get functions.

The code I have shown belongs in your operator+.
Why is that better than the code that I have, since they do the same thing.

 ``1234`` `````` for(int i = 0; i < index; i++) { polygon[i] = _point[i] + point; }``````
Last edited on
 I also fixed the error which only copied the first point to the polygon and left all the other ones at their default values..

cool, I don't know how I missed that in spite of compiling your code .. So, everything's working fine now ? :)

Edit:
you may want to post the complete working code ... it will help anyone who might stumble across this post later via google, etc.
Last edited on
Haven't fixed the polygon + polygon yet but will edit it in there once I get it to work
properly
 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149`` ``````#include using namespace std; class Point { public: Point(float _x=0, float _y=0) // constructor { x = _x, y=_y; } Point operator + (const Point &point { float _x = x + point.x; float _y = y + point.y; return Point(_x,_y); } Point operator - (const Point &point) { float _x = x - point.x; float _y = y - point.y; return Point(_x,_y); } Point operator * (const Point &point) { float _x = (x * point.x) - (y * point.y); float _y = (x * point.y) + (y * point.x); return Point(_x,_y); } friend ostream& operator << (ostream& os,Point pt); private: float x; float y; }; class Polygon { public: Polygon(int i = 0) //Default constructor { _point = new Point[i]; index = i; } Polygon operator + (const Polygon &polygon) { //some code } Polygon operator + (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { polygon[i] = _point[i] + point; } return polygon; } Point& operator [](int i) { return _point[i]; } Polygon operator * (const Point &point) { Polygon polygon(index); for(int i = 0; i < index; i++) { polygon[i] = polygon[i] * point; } return polygon; } Polygon& operator =(const Polygon &polygon) { delete[] _point; index = polygon.index; _point = new Point[index]; for(int i = 0; i< index; i++) { _point[i] = polygon._point[i]; } return *this; } Polygon(const Polygon &polygon) // Copy constructor { index = polygon.index; _point = new Point[index]; for(int i = 0; i< index; i++) { _point[i] = (polygon._point)[i]; } cout << "\tcopy constructor invoked .." << endl; } ~Polygon() { delete [] _point; } friend ostream& operator << (ostream& os, Polygon polygon); private: Point *_point; int index; }; ostream& operator<<(ostream& os, Point point) { os << point.x << "," << point.y; return os; } ostream& operator <<(ostream& os, Polygon polygon) { for(int i = 0; i < polygon.index ;i++) { os << polygon[i] << endl; } return os; } int main() { Polygon polygon1(3); polygon1[0] = Point(0,0); polygon1[1] = Point(1,1); polygon1[2] = Point(2,2); Point point1(6,5); // Testing copy constructor cout << polygon1[0] << endl; cout << polygon1[1] << endl; cout << polygon1[2] << endl; cout << endl; Polygon p1 = polygon1; // This invokes the copy constructor cout << p1[0] << endl; cout << p1[1] << endl; cout << p1[2] << endl; cout << endl; polygon1 = polygon1 + point1; cout << polygon1 << endl; system("pause"); return 0; }``````

Last edited on
Topic archived. No new replies allowed.