Why can't I push this object to my std :: list?

Question

Just started programming in C ++.

I created a Point class, std :: list and an iterator:

class Point { public: int x, y; Point(int x1, int y1) { x = x1; y = y1; } }; std::list <Point> pointList; std::list <Point>::iterator iter; 

Then I click the new points on the pointList.

Now I need to iterate over all the points in the pointList, so I need to use a loop using an iterator. I'm screwing up here.

 for(iter = pointList.begin(); iter != pointList.end(); iter++) { Point currentPoint = *iter; glVertex2i(currentPoint.x, currentPoint.y); } 

Strike>

---

Update

You guys were right, the problem is that I do not iterate over the list. It seems like the problem is that I'm trying to push something to the list.

Exact error:

mouse.cpp: In the function void mouseHandler(int, int, int, int)': mouse.cpp:59: error: conversion from Point *' for a non-scalar type, a "point" is requested

These lines are:

  if (button == GLUT_LEFT_BUTTON && state == GLUT_DOWN) { Point currentPoint = new Point(x, y); pointList.push_front(currentPoint); } 

What does it mean to convert a Point * to a non-scalar Point type? I am just trying to create new points and paste them into the list here.

Answer 1

This must be a valid bit of code.

 #include <iostream> #include <list> class Point { public: int x, y; Point(int x1, int y1) { x = x1; y = y1; } }; int main() { std::list<Point> points; points.push_back(Point(0, 0)); points.push_back(Point(1, 1)); points.push_back(Point(2, 2)); std::list<Point>::iterator iter; for(iter = points.begin(); iter != points.end(); ++iter) { Point test = *iter; std::cout << test.x << ", " << test.y << "; "; } std::cout << std::endl; return 0; } 

Using this code:

 jasons-macbook41:~ g++ test.cpp jasons-macbook41:~ ./a.out 0, 0; 1, 1; 2, 2; jasons-macbook41:~ 

Although I would not create a temporary copy of the Point as a code. I would rewrite the loop as follows:

 for(iter = points.begin(); iter != points.end(); ++iter) { std::cout << iter->x << ", " << iter->y << "; "; } 

An iterator is syntactically like a pointer.

EDIT: Given your new problem, drop the “new” off the construction line. This creates a pointer to a point, as opposed to a point on the stack. That would be true:

 Point* temp = new Point(0, 0); 

Or that:

 Point temp = Point(0, 0); 

And you will be better off with the latter.

Answer 2

a few things.

• Have you tried iter->x and iter->y instead of copying the value?
• The error you are talking about is hard to understand. You are not trying to get x and y through the iterator, you are copying the iterator data to a new point.

EDIT:

according to new information in OP. You try to create a new object without a pointer, and then try to fill the point with a vector that accepts only objects. You either need to make the vector a vector of pointers and do not forget to delete their afterwords or create a new point on the stack and copy them into a vector with a standard purpose. Try the following:

 if (button == GLUT_LEFT_BUTTON && state == GLUT_DOWN) { Point currentPoint = Point(x, y); pointList.push_front(currentPoint); } 

Answer 3

If you already have a function that you want to apply to the entire list, the path std :: for_each is a path, for example,

 std::for_each(pointList.begin(), pointList.end(), myGreatFunction); 

If you need to write a for loop, then something like this:

 std::list<Point>::iterator itEnd = pointList.end(); for(std::list<Point>::iterator itCur=pointList.begin(); itCur != itEnd; ++itCur) { yourFunction(itCur->x, itCur->y); } 

Notes:

• ++ itCur may be more efficient than itCur ++ due to return types (reference vs value / copy)

Answer 4

The non-scalar problem is that you assign a point pointer (the return value of the new operator) to the point stack object (since it is not a * point in your code).

I would recommend saying

  Point currentPoint(x, y); pointList.push_front(currentPoint); 

Note that currentPoint will be copied to your list; the implicitly created Point copy constructor (since you did not declare the Point (const Point & other) constructor in your class, the compiler did this for you) will copy currentPoint.x and currentPoint.y to the list; in this case, it’s good. The point is small, so the copy consumption is low, and it contains only two ints, so direct copying of ints is fine.

Answer 5

This answer relates to an edited version of the question.

As gbrandt said in the edited version of his answer, your problem is that you are trying to dynamically allocate an instance of Point , and then assign it to a Point object, not a pointer to a Point . The result of new is a pointer to a Point , not a Point object - what you actually want in this case is the last one that you create without new :

 Point currentPoint(x, y); pointList.push_front(currentPoint); 

Since list<T>::push_front() clicks a copy of the Point object in the list, you do not need to do dynamic allocation here. It is much safer to avoid dynamic allocation when possible, as this can easily lead to memory leaks - for example, the following alternative code that compiles and runs causes a memory leak because the object pointed to by currentPoint never delete d:

 Point *currentPoint = new Point(x, y); pointList.push_front(*currentPoint); // Notice the "*" 

Of course, you can simply add delete currentPoint; in the end, to remove the leak, but why use slow dynamic allocation when stack-based allocation makes work faster and easier?

Answer 6

This is how I usually approach these loops if you don't want to use std :: foreach:

 for (iter curr = pointListObject.begin(), end = pointListObject.end(); curr != end; ++curr) { glVertex2i(curr->x, curr->y); } 

Be careful with these points:

• pointListObject is an instance of pointList; if you use a class (type pointList, not an instance of pointList), you are in troble, but the compiler roars a lot. Same thing with the iterator. It just makes searching easier if you keep the type names and the names of the individual elements.
• Performing joint initialization of iterators like this allows you to save the initialization of the end within the loop (useful for defining the scope), while at the same time keeping the loop running cheap.

Answer 7

Did you cut and paste this code in SO from your .cpp file, or did you reuse it? From the sound of your error message, I would suggest that the source code says

 glVertex2i(iter.x, iter.y); 

Which, as the grandtt pointed out, is not correctly looking for an iterator.

I would rewrite the loop as follows:

 std::list<Point>::const_iterator iter = pointList.begin(); const std::list<Point>::const_iterator end = pointList.end(); for (; iter != end; ++iter) { const Point& p = *iter; glVertex2i(px, py); } 

The main changes are to use const_iterators instead of not-const, since your loop is not intended to change the contents of a list. Then take the begin () and end () values ​​exactly once, use preincrement and find the iterator once in the const reference. That way, you don't have a copy where your source code copied the Point object referenced by * iter, and you avoid dereferencing the iterator twice for about the same efficiency as you can get here.

Now, for some unsolicited OpenGL tips, I would also point out that vertex arrays are probably a better choice than immediate mode calls (glVertex *).

Hope this helps ...