C ++ std :: sort не работает с указателем на вектор - PullRequest
1 голос
/ 10 января 2011

После долгих поисков кажется, что указатель на вектор - не лучшая вещь.Тем не менее, следующий код вызывает у меня слишком много проблем:

  1 #include <stdio.h>
  2 #include <algorithm>
  3 #include <vector>
  4
  5
  6 class Hdr
  7 {
  8     public:
  9     std::vector<long> *order;
 10     bool operator()(long i1, long i2) const;
 11     Hdr(int N);
 12     ~Hdr();
 13 };
 14
 15 Hdr::Hdr(int N)
 16 {
 17     order = new std::vector<long>(N,0);
 18     for(int k=0;k<N;k++) (*order)[k] = -k;
 19 };
 20
 21 Hdr::~Hdr()
 22 {
 23     order->clear();
 24     delete order;
 25 };
 26
 27 bool Hdr::operator()(long i1, long i2) const
 28 {
 29     return (i1<i2);
 30 };
 31
 32 int main(void)
 33 {
 34     Hdr mhdr(1000);
 35     std::sort(mhdr.order->begin(),mhdr.order->end(),mhdr);
 36
 37     printf("value at 300 = %d\n",mhdr.order->at(300));
 38 };

с gcc 4.3 в Linux, исполняемый файл выдает «двойное освобождение или повреждение».Поэтому я закомментировал строку 24, и она выдает 'std :: out_of_range'.Видимо, строка 35 (sort) все испортила, когда динамически размещенный вектор передается в std :: sort.Или у меня где-то была большая ошибка.Пожалуйста помоги!!Если я изменю строку 9 на std :: vector, тогда все будет хорошо;однако я не могу продолжать задаваться вопросом, что пошло не так с указателем на вектор.

Ответы [ 5 ]

12 голосов
/ 10 января 2011

Вы не следуете правилу из трех : вы не определили конструктор копирования и оператор присваивания для своего класса.

Вы делаете копию mhdr, когда передаете ее в качестве функтора на std::sort. Элемент данных order этой копии и оригинала mhdr указывают на один и тот же объект. Копия уничтожается, в результате чего объект, на который указывает его order, удаляется, а затем, когда заканчивается main, оригинал уничтожается, что вызывает двойное удаление, неопределенное поведение и другие забавные вещи.

(На самом деле, внутри std::sort может быть сделано еще больше копий, поэтому он может рухнуть еще раньше.)

Почему вы динамически распределяете std::vector? Если для этого есть веские причины, их очень мало.

5 голосов
/ 10 января 2011

Вы передаете mhdr по значению std::sort(), с конструктором копирования по умолчанию, который дублирует указатель на ваш вектор, и delete s, когда этот экземпляр выходит из области видимости.Это не то, что вам нужно.

Вы, вероятно, должны использовать объект компаратора, отличный от объекта, содержащего этот вектор, или ссылаться на указатель на вектор (возможно, используя соответствующий класс интеллектуальных указателей).

2 голосов
/ 10 января 2011

Вам не нужно bool operator()() для этого класса - long s можно отсортировать со сравнением по умолчанию (см. std::sort). Проблема, сообщаемая во время выполнения, заключается в том, что третий аргумент скопирован , и у вас нет ни конструктора копирования, ни оператора копирования в Hdr (см. правило трех ).

1 голос
/ 10 января 2011

Проблема в том, что std :: sort принимает функтор сравнения по значению, поэтому деструктор будет вызываться для копии и для исходного объекта mhdr, и, таким образом, поскольку вы не предоставили конструктор копирования, orderбудет удален дважды.

Самое простое решение - использовать обычный std::vector вместо указателя на std::vector.Делая это, неявный конструктор копирования будет работать для вас.

Кроме того, вы можете определить свой конструктор копирования и сделать глубокую копию вектора order или использовать умный указатель, чтобы обернуть его.

Но лучшим решением было бы сделать функцию сравнения статической функцией-членом (или глобальной функцией) и использовать ее в качестве параметра для std::sort.Вы всегда должны использовать специальные классы функторов.Не добавляйте operator() к классу, который уже делает что-то еще.Он может работать так, как ожидалось, но, скорее всего, он не будет таким же эффективным, как выделенный класс функторов, поскольку может потребоваться гораздо больше операций копирования (для переменных-членов, как в этом случае).Конечно, вам все равно следует исправить конструктор копирования класса Hdr (или просто объявить копировальный ctor без определения, чтобы удалить неявный, который в этом случае не работает).

0 голосов
/ 11 января 2011

Как уже упоминалось, главная причина этого в том, что когда вы передаете mhdr в качестве компаратора, вы делаете копию.Эта копия содержит указатель на тот же вектор.Когда эта копия уничтожается, этот вектор уничтожается, разрушая коллекцию, которая была между вашими итераторами.

Это было бы очевидно, если бы вы предотвратили копирование и присвоение, объявив эти операторы частными и не реализовав их.

Причина, по которой вы попали в этот беспорядок, заключается в том, что ваш класс Hdr имеет две разные функции - поддержание вектора и наличие функции сравнения.Классы должны делать одно и одно хорошо.

Решение, приведенное ниже, исправляет эти проблемы, а также некоторые другие проблемы, такие как представление элементов данных как public (хотя я сохранил указатель на vector, так какЯ подозреваю, что вы пытались проиллюстрировать что-то об этом деле, хотя я согласен с другими ответчиками, что это сомнительное решение)

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

class Hdr
{
    public:
          Hdr(int N);
        ~Hdr();

        typedef std::vector<long>::iterator OrderIt;
        OrderIt orderBegin();
        OrderIt orderEnd();
        long orderAt(int index) const;

    private:
        Hdr& operator=(const Hdr&);
        Hdr(const Hdr&);

        std::auto_ptr<std::vector<long> > order;
};

class Comparator
{
    public:
        bool operator()(long i1, long i2) const;

};

Hdr::Hdr(int N)
{
    order = std::auto_ptr<std::vector<long> >(new std::vector<long>(N,0));
    for(int k=0;k<N;k++) 
    {
        (*order)[k] = -k;
    }
};

Hdr::~Hdr()
{
    order->clear();
};

Hdr::OrderIt Hdr::orderBegin()
{
    return order->begin();
}

Hdr::OrderIt Hdr::orderEnd()
{
    return order->end();
}

long Hdr::orderAt(int nIndex) const
{
    return order->at(nIndex);
}

bool Comparator::operator()(long i1, long i2) const
{
    return (i1<i2);
};

int main(void)
{
    Hdr mhdr(1000);
    std::sort(mhdr.orderBegin(),mhdr.orderEnd(), Comparator());
    printf("value at 300 = %d\n",mhdr.orderAt(300));
};
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...