How am I accidentally overwriting when referencing these pointers?

Question

Last question for tonight, I promise. These pointers are giving me a serious headache.

I have a std::list<Point> called Polygon and a std::list of Polygons defined like:

typedef std::list<Point> Polygon; 
typedef std::list<Polygon> PolygonList; 

// List of all our polygons 
PolygonList polygonList;

I created the method below to attempt to delete the nearest Point from an (x,y), checking all of my Polygons within my polygonList.

void deleteNearestPoint(int x, int y)
{
    y = screenHeight - y;

    Polygon &closestPolygon = polygonList.front();
    Polygon::iterator closestPoint = closestPolygon.begin();

    float closestDistance = sqrt(pow(x - closestPoint->x, 2) + pow(y - closestPoint->y, 2));

    // Search PolygonList
    PolygonList::iterator listIter;
    Polygon::iterator iter;

    for(listIter = polygonList.begin(); listIter != polygonList.end(); listIter++)
    {
        Polygon &tempPolygon = *listIter;

        for(iter = tempPolygon.begin(); iter != tempPolygon.end(); iter++)
        {
            const float distance = sqrt(pow(x - iter->x, 2) + pow(y - iter->y, 2));

            if (distance < closestDistance)
            {
                closestPolygon = *listIter;
                closestPoint = iter;
                closestDistance = distance;
            }
        }

    }

    closestPolygon.erase(closestPoint);

    redraw();
}

However, somewhere I have a pointer or reference variable screwing me up. This code compiles but acts in a very strange way.

I've written a debug statement and lets say I have 3 polygons in my polygon list like so:

Polygon #: 0
Point: (448, 43)
Point: (469, 177)
Point: (374, 123)
Polygon #: 1
Point: (295, 360)
Point: (422, 350)
Point: (315, 266)
Point: (295, 360)
Polygon #: 2
Point: (143, 202)
Point: (301, 203)
Point: (222, 100)
Point: (143, 202)

Now, lets say I try and use the delete function giving it an x/y close to point 422, 350 The desired result would be it simply deleting that point (422, 350) from Polygon #1 but instead I get this:

Polygon #: 0
Point: (295, 360)
Point: (422, 350)
Point: (315, 266)
Point: (295, 360)
Polygon #: 1
Point: (295, 360)
Point: (315, 266)
Point: (295, 360)
Polygon #: 2
Point: (143, 202)
Point: (301, 203)
Point: (222, 100)
Point: (143, 202)

It did delete (422, 350) but it also has the strange side effect of overwriting Polygon#0 to what Polygon#1 was before the delete of its point.

I know I'm using a pointer or reference incorrectly in my method. Can someone point out what I could possibly be doing that is causing this? I think it is because my &closestPolygon is declared as a reference, but I get compile errors if I try to set it as anything else.

Answer 1

You can't assign to a reference.
This is a main difference between references in C++ and references in C# and the like. Once you initialize a reference variable with an object, that variable becomes an alias to that object. assigning to the reference variable is equivalent to assigning to the original object. with this in mind, this line:

closestPolygon = *listIter;

means that you're overwriting the previously found closest polygon with the current one. This is also consistent with the result you're getting. I suggest you use pointers instead of reference for this method.

Also, as noted by someone else, using pow(... , 2) is extremely wasteful. better yet write something like this:

x = a - b;
xsquare = x * x;

---

EDIT: Writing this with pointers will start with something like that:

void deleteNearestPoint(int x, int y)
{
    y = screenHeight - y;

    Polygon *closestPolygon = &polygonList.front();
    Polygon::iterator closestPoint = closestPolygon.begin();

Answer 2

Unfortunately, you cannot rebind a reference, i.e., this line:

closestPolygon = *listIter;

will copy *listIter to closestPolygon, not rebind the refernece to *listIter.

Edit: To do what you want, you should use PolygonList::iterator instead of Polygon & and adjust the code accordingly.

Answer 3

closestPolygon = *listIter;

Will instead call operator=() on the object pointed by the reference, so it will overwrite the first polygon with the second.

Declare closestPolygon as a pointer instead (which can pointed to different objects even after its declaration)

Polygon *closestPolygon = &(polygonList.front());
...
closestPolygon = &(*listIter);

Answer 4

I see your question has already been answered - I'll throw my 2 cents in and suggest you make distance a method of Point to save duplicating code.

As others have said you might also want a squareDistance(x, y) method as well if that'll make life easier for you.

Either could be inlined if you're worried about the overhead of the function call.

Answer 5

Other answers have pointed out what caused the error. As a general advice I would suggest not using references except in function arguments. The semantics are confusing, also for someone that will try to read your code. Try rewriting to something like this (I didn't test the code):

void deleteNearestPoint(int x, int y)
{
    y = screenHeight - y;

    PolygonList::iterator closestPolygon = polygonList.begin();
    Polygon::iterator closestPoint = closestPolygon->begin();

    float closestDistance = sqrt(pow(x - closestPoint->x, 2) + pow(y - closestPoint->y, 2));

    // Search PolygonList
    PolygonList::iterator listIter;
    Polygon::iterator iter;

    for(listIter = polygonList.begin(); listIter != polygonList.end(); listIter++)
    {
        for(iter = listIter->begin(); iter != listIter->end(); iter++)
        {
            const float distance = sqrt(pow(x - iter->x, 2) + pow(y - iter->y, 2));

            if (distance < closestDistance)
            {
                closestPolygon = listIter;
                closestPoint = iter;
                closestDistance = distance;
            }
        }

    }

    closestPolygon->erase(closestPoint);

    redraw();
}