2015-10-18 4 views
2

У меня есть объект Holder с этими тремя функциямиудаление unique_ptr объекта из вектора значения атрибута

unique_ptr<Object> Holder::remove(string objName){ 
    std::vector<unique_ptr<Object>>::iterator object = 
      find_if(objects.begin(), objects.end(), 
      [&](unique_ptr<Object> & obj){ return obj->name() == objName;} 
    ); 
    objects.erase(std::remove(objects.begin(), objects.end(), *object)); 
    return std::move(*object); 
} 

vector<unique_ptr<Object>> const& Holder::getContent() const { 
    return this->objects; 
} 

void Holder::add(unique_ptr<Object> objPtr) { 
    this->objects.push_back(move(objPtr)); 
} 

Я написал тест CppUnit, как показано ниже:

void HolderTest::removeObject() { 
    Holder holder("bag"); 
    unique_ptr<Object> ringPtr(new Object("a")); 
    holder.add(move(ringPtr)); 

    unique_ptr<Object> swordPtr(new Object("b")); 
    holder.add(move(swordPtr)); 

    holder.remove("a"); 
    vector<unique_ptr<Object>> const& objects = holder.getContent(); 
    CPPUNIT_ASSERT(objects.size() == 1); 
} 

Этот тест проходит без проблем, но для меня очень странно, что если я добавлю следующую строку:

const std::string name = objects[0].get()->name(); 
CPPUNIT_ASSERT_EQUALS("b", name); 

Затем тест сбой без какого-либо сообщения. Я написал эту строку в другом тесте без вызова remove и работает без каких-либо проблем. Если я изменяю значение размера вектора на два или 0 CPPUNIT_ASSERT (objects.size() == 2); Затем тест не удался. Так кажется, что функция remove сохраняет один из unique_ptr, но превращает его в nullptr? Любая проблема в том, что это проблема?

+0

Ваш итератор "объект" после этого строка не действительна: objects.erase (std :: remove (objects.begin(), objects.end(), * object)); Это делает строку «return std :: move (* object)»; вызывает неопределенное поведение и все испортит. – Gene

ответ

1
std::vector<unique_ptr<Object>>::iterator object = 
     find_if(objects.begin(), objects.end(), 
       [&](unique_ptr<Object> & obj){ return obj->name() == objName;} 
       ); 
    objects.erase(std::remove(objects.begin(), objects.end(), *object)); 
    return std::move(*object); 

Вы derefence итератор objectпосле он был недействительным. См. Iterator invalidation rules

Переместите указатель перед стиранием, тогда с вами все будет в порядке.

Другие ноты:

  • это смешно использовать removing со значением (вместо того, чтобы просто удаление итератор вы получили). Вы ожидаете, что вектор будет содержать дубликаты? На самом деле, удалите это: это сделает неправильным erase, потому что он всегда удаляет один элемент.
  • вы также не проверяете, что object может быть итератором end() до разыменования. Другой источник Undefined Behaviour
  • рассмотреть вопрос о принятии name по const& для повышения эффективности

Live On Coliru

#include <memory> 
#include <vector> 
#include <iostream> 
#include <algorithm> 

using namespace std; 

struct Object { 
    Object(std::string name) : _name(std::move(name)) { } 

    std::string const& name() const { return _name; } 
    private: 
    std::string _name; 
}; 

struct Holder { 
    using Ptr = unique_ptr<Object>; 

    Ptr remove(string const& objName) { 

     auto it = find_if(objects.begin(), objects.end(), [&](Ptr& obj){ return obj->name() == objName; }); 

     if (it != objects.end()) { 
      auto retval = std::move(*it); 
      objects.erase(it); 
      return std::move(retval); 
     } 

     return {}; // or handle as error? 
    } 

    vector<Ptr> const& getContent() const { 
     return this->objects; 
    } 

    void add(Ptr objPtr) { 
     this->objects.push_back(move(objPtr)); 
    } 

    private: 
    vector<Ptr> objects; 
}; 

int main() { 

    Holder h; 
    for(auto n: { "aap", "noot", "mies", "broer", "zus", "jet" }) 
     h.add(std::make_unique<Object>(n)); 

    h.remove("broer"); 
    h.remove("zus"); 

    for (auto& o : h.getContent()) 
     std::cout << o->name() << "\n"; 
} 

Печать

aap 
noot 
mies 
jet 
+0

Я действительно не понял, что вы имеете в виду? Вы имеете в виду: Я должен назначить std :: move (* object) переменной, и тогда я должен вернуть эту переменную в return? – Govan

+0

@Govan Я добавил полный, самодостаточный образец, который должен помочь вам понять – sehe

+0

Спасибо! Теперь я проблема. – Govan

Смежные вопросы