C++ std::sort() Calling Destructor

Question

I overloaded my class' () operator to use it as a sort comparer function. When using std::sort(), it for some reason calls the the class' destructor a bunch of times (dependant on the amount of entries in the vector, apparently). I've described more in ~RANK().

#include <stdio.h>
#include <stdlib.h>
#include <vector>
#include <algorithm>

class RANK
{
    struct COMBO
    {
        int x;
    };

    std::vector<COMBO *> data;
public:
    RANK()
    {
        printf("RANK()\n");
    }

    ~RANK()
    {
        printf("~RANK()\n");

        /*
         * Here is the problem.
         * Since my vector consists of pointers to COMBO objects,
         * I delete them upon RANK object's destruction. However,
         * std::sort() calls RANK's destructor many times and
         * throws some runtime error, unless commented out.
         */
        //for (unsigned int i = 0, n = data.size(); i < n; i++)
        //  delete data[i];
    }

    void Add(int x)
    {
        COMBO *combo = new COMBO();
        combo->x = x;

        data.push_back(combo);
    }

    unsigned int Size()
    {
        return data.size();
    }

    void Sort()
    {
        std::sort(data.begin(), data.end(), *this);
    }

    int operator[](unsigned int pos)
    {
        return data[pos]->x;
    }

    bool operator()(COMBO *combo1, COMBO *combo2)
    {
        return combo1->x > combo2->x;
    }
};

int main()
{
    RANK rank;
    rank.Add(1337);
    rank.Add(9001);
    rank.Sort();

    for (unsigned int i = 0, n = rank.Size(); i < n; i++)
        printf("%d ", rank[i]);
        printf("\n");

    system("pause");

    return 0;
}

The output (with commented destructor):

RANK()
~RANK()
~RANK()
~RANK()
~RANK()
~RANK()
9001 1337

score 6 · Answer 1 · answered Nov 14 '12 at 18:09

6

The comparison function to std::sort is passed by value. By using the RANK object as the comparator, you are passing a copy to std::sort (as the last value) and it may copy it more than once internally.

I would suggest separating out the comparison operator for COMBO from the class RANK

answered Nov 14 '12 at 18:09

Marshall Clow

15,972
2
29
45

1

Yes, and because all copies hold pointers to the same objects, you get the runtime error when the second time a destructor of one of the copies is called. – Axel Nov 14 '12 at 18:14

score 2 · Accepted Answer · edited May 23 '17 at 11:48

The first problem is that you're breaking the Rule of Three. Your class requires a non-trivial destructor to release its resources, so it needs to be either correctly copyable or uncopyable to avoid multiple objects owning the same resources. The simplest solution is to prevent copying by deleting the copy constructor and copy-assignment operator:

RANK(RANK const &) = delete;
void operator=(RANK const &) = delete;

or, if you're stuck with a pre-2011 compiler, declare them private with no implementation.

Alternatively, you could consider storing smart pointers such as std::unique_ptr (to prevent copying) or std::shared_ptr (to allow shared ownership); if you do that, then your class will have the same (safe) copy semantics as the pointer you choose.

Preventing copying will make the second problem obvious: you're using the RANK object as the comparator for std::sort. The comparator is taken by value, so the object is copied there. That's easy to fix by defining a separate type for the comparator:

struct CompareCOMBO {
    bool operator()(COMBO *combo1, COMBO *combo2) {
       return combol1->x > combo2->x;
    }
};

std::sort(data.begin(), data.end(), CompareCOMBO());

or, if you can use lambdas:

std::sort(data.begin(), data.end(), 
    [](COMBO *combo1, COMBO *combo2){
         return combo1->x > combo2->x;
    }
);

Added a copy constructor - solved the problem. Switched to a comparator struct so it wouldn't mess with RANK's destructor at all - even better. Realized about lambdas - dayum, son! — hleV, Nov 14 '12 at 19:19

score 1 · Answer 3 · answered Nov 14 '12 at 18:07

1

Provide a copy constructor and place a breakpoint inside to see where it is called.

answered Nov 14 '12 at 18:07

Axel

13,939
5
50
79

score 0 · Answer 4 · edited May 23 '17 at 12:18

0

By passing *this as your compare object it gets copied during the sort (that's why you don't see the construction popping, if you had a copy constructor you would see calls to that).

Consider this thread to sort your objects

edited May 23 '17 at 12:18

Community

1
1

answered Nov 14 '12 at 18:14

emartel

7,712
1
30
58

C++ std::sort() Calling Destructor

4 Answers4