Pointer and QVector issue

Question

I want to define functions which will delete self-defined type object and the index from the QVector. Originally the source was as follows:

    Point PointCollection::RemovePoint(int index)
{
    Point removedPoint = new Point(this[index].Id, this[index].X, this[index].Y);
    this->remove(index);
    updateCentroid();
    return (removedPoint);
}

Point PointCollection::RemovePoint(Point p)
{
    Point removedPoint = new Point(p.GetId(), p.GetX(), p.GetY());
    this.remove(p);
    updateCentroid();
    return (removedPoint);
}

which was not working as I think because of new. Then I modified the source to the following:

Point PointCollection::deletePoint(int Index)
{
    Point deleted = Point(this[Index].Id, this[Index].X, this[Index].Y);
    this->remove(Index);
    updateCentroid();
    return(deleted);
}

Point PointCollection::deletePoint(Point point)
{
    Point deleted = Point(point.GetId(), point.GetX(), point.GetY());
    this->remove(point);
    updateCentroid();
    return(deleted);
}

Now Point PointCollection::deletePoint(int Index) compiles without any error, but this->remove(point); in Point PointCollection::deletePoint(Point point) functioned is compiled with following error:

error: no matching function for call to 'PointCollection::remove(Point&)'

Q1: Did I do correct that removed new? Q2: How to fix the error I am having.

`Point removedPoint = new Point` ? Shouldn't that be a pointer? Lots of stuff wrong with your code. What is a "self defined type object"? — dtech, Aug 20 '13 at 19:07
You cannot say `this[someIndex]` (well, you can, but it will lead to disaster). — juanchopanza, Aug 20 '13 at 19:08
@ddriver "self defined type object" - I mean the type object is Point, the type Point is defined by me. | You mean Point *removedPoint = new Point ? — Mike, Aug 20 '13 at 19:13
@juanchopanza still it compiles fine. Why should it lead to disaster? what is an alternative? — Mike, Aug 20 '13 at 19:14
@Mike - yes, the `new` operator returns a pointer to the allocated object. Also, self-defined type object to me sounds like the object defines its type itself... certainly not the most clear form of speech. `this[index]` is dangerous because it goes to an arbitrary address in the blind. `this` is a pointer but not an array. You can still use offsets, but that's what member identifiers are for, so you don't do it in the blind like that. — dtech, Aug 20 '13 at 19:15
@ddriver I am sorry if in my question are parts, which can lead to misunderstanding it. I tried also to use a pointer in definition, but I had error when returning the value. So, will it bother you, if I ask you to modify my two functions the way you think it will work? Thanks — Mike, Aug 20 '13 at 19:19
Perhaps it would be more productive to learn C++ before attempting anything even moderately complex. See [this lise of books](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to get started. — juanchopanza, Aug 20 '13 at 19:44

dtech · Accepted Answer · 2013-08-20T20:15:13.170

Your approach seems to be overall wrong. First of all focus on what you need:

performance and memory efficiency or...
fast inserts and deletes

QVector is of the former kind. If you do deletes and inserts that are not in the back you will likely get poor performance. Because the entire vector has to be reallocated every time you do a change.

If you need to insert and delete often go for a linked list, for example QLinkedList.

Qt already provides containers, implementing your own doesn't offer much benefit, it is not likely that you will produce a better container than a bunch of professionals working on this framework for 20 years.

Here is a simple snippet how to insert and delete points in vector and linked list. You can use this methods to implement your own wrapper class if you want:

    QVector<QPoint> myPointVector;
    QLinkedList<QPoint> myPointList;

    // push back some data
    myPointVector << QPoint(1, 1) << QPoint(2, 2) << QPoint(3, 3) << QPoint(4, 4);
    myPointList << QPoint(1, 1) << QPoint(2, 2) << QPoint(3, 3) << QPoint(4, 4);

    foreach (QPoint p, myPointVector) qDebug() << p;
    foreach (QPoint p, myPointList) qDebug() << p;
    qDebug() << endl;    

    auto i1 = myPointVector.indexOf(QPoint(2, 2));
    auto i2 = qFind(myPointList.begin(), myPointList.end(), QPoint(2,2));

    myPointVector.insert(i1, QPoint(5,5)); // or existing point object / reference
    auto i3 = myPointList.insert(i2, QPoint(5,5));

    foreach (QPoint p, myPointVector) qDebug() << p;
    foreach (QPoint p, myPointList) qDebug() << p;
    qDebug() << endl;

    QPoint deletedFromVector = myPointVector[i1]; // use those to return before deleting
    QPoint deletedFromList = *i3;  // note you don't need to construct just assign  

    myPointVector.remove(i1);
    myPointList.erase(i3);

    foreach (QPoint p, myPointVector) qDebug() << p;
    foreach (QPoint p, myPointList) qDebug() << p;
    qDebug() << endl;

As you see, initially both containers contain points 1 2 3 4, then point 5 is inserted in the place of 2, then removed again. The vector uses an integer index for the operations, the list uses an iterator. That is why when 5 is inserted, I get its "index" because unlike the vector it won't push back the rest, so if i2 is removed it will not remove point 5 which was inserted in place of point 2, but point 2 to which it still refers to.

Also, if you want to insert in the list in a given index, you can just use the begin iterator + index to "forward" the iterator the appropriate number of positions.

Hopefully that made sense. Naturally you can use you point class in the place of QPoint.

Pointer and QVector issue

1 Answers1