shared_ptr, который удаляет себя из контейнера, которому он принадлежит, есть ли лучший способ?

Question

То, что я хочу сделать, - это поставить группу объектов задачи в контейнер, где задача может удалить себя из очереди. Но я также не хочу, чтобы объект уничтожался при удалении, поэтому он может продолжать завершать sh независимо от выполняемой работы.

Итак, безопасный способ сделать это - либо вызвать RemoveSelf() когда работа сделана, или используйте ссылку keepAlive, затем продолжайте работу. Я убедился, что это действительно работает, в то время как DoWorkUnsafe всегда будет обрабатывать sh после нескольких итераций.

Я не особенно доволен решением, потому что я должен либо не забыть позвонить RemoveSelf() в конце выполняемой работы, или не забудьте использовать keepAlive, в противном случае это приведет к неопределенному поведению.

Другая проблема заключается в том, что если кто-то решит перебрать ownerList и выполнить работу , он лишит законной силы итератор при их итерации, что также небезопасно.

В качестве альтернативы я знаю, что вместо этого я могу поместить задачу в отдельную очередь «очистки» и уничтожить завершенные задачи по отдельности. Но этот метод мне показался более точным, но со слишком большим количеством оговорок.

Есть ли лучший шаблон для обработки чего-то подобного?

#include <memory>
#include <unordered_set>

class SelfDestruct : public std::enable_shared_from_this<SelfDestruct> {
public:
    SelfDestruct(std::unordered_set<std::shared_ptr<SelfDestruct>> &ownerSet)
        : _ownerSet(ownerSet){}

    void DoWorkUnsafe() {
        RemoveSelf();
        DoWork();
    }

    void DoWorkSafe() {
        DoWork();
        RemoveSelf();
    }

    void DoWorkAlsoSafe() {
        auto keepAlive = RemoveSelf();
        DoWork();
    }


    std::shared_ptr<SelfDestruct> RemoveSelf() { 
        auto keepAlive = shared_from_this();
        _ownerSet.erase(keepAlive);
        return keepAlive;
    };

private:

    void DoWork() {
        for (auto i = 0; i < 100; ++i)
            _dummy.push_back(i);
    }

    std::unordered_set<std::shared_ptr<SelfDestruct>> &_ownerSet;
    std::vector<int> _dummy;
};

TEST_CASE("Self destruct should not cause undefined behavior") {

    std::unordered_set<std::shared_ptr<SelfDestruct>> ownerSet;

    for (auto i = 0; i < 100; ++i)
        ownerSet.emplace(std::make_shared<SelfDestruct>(ownerSet));

    while (!ownerSet.empty()) {
        (*ownerSet.begin())->DoWorkSafe();
    }
}

Answer 1

Существует хороший принцип проектирования, согласно которому у каждого класса должна быть одна цель. Для выполнения этой задачи должен существовать «объект задачи». Когда вы начинаете добавлять дополнительные обязанности, вы, как правило, оказываются в беспорядке. Беспорядки могут включать в себя необходимость не забывать вызывать определенный метод после выполнения основной задачи или необходимость не использовать хакерский обходной путь, чтобы сохранить объект живым. Беспорядки часто являются признаком неадекватной мысли, заложенной в дизайн. Недовольство беспорядком говорит о вашем потенциале для хорошего дизайна.

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

Так что мы заново представим ваш класс без этого беспорядка "SelfDestruct". Ваши объекты задачи существуют для выполнения задачи. Они, вероятно, полиморфные c, поэтому необходим контейнер указателей на объекты задач, а не контейнер объектов задач. Объекты задачи не заботятся о том, как они управляются; это работа для кого-то другого.

class Task {
public:
    Task() {}
    // Other constructors, the destructor, assignment operators, etc. go here

    void DoWork() {
        // Stuff is done here.
        // The work might involve adding tasks to the queue.
    }
};

Теперь сосредоточьтесь на контейнере. Контейнер (точнее, владелец контейнера) отвечает за добавление и удаление элементов. Так сделай это. Вы, кажется, предпочитаете удалить элемент, прежде чем вызывать его. Это кажется хорошей идеей для меня, но не пытайтесь отложить удаление задачи. Вместо этого используйте вспомогательную функцию, сохраняя эту логику c на уровне абстракции владельца контейнера.

// Extract the first element of `ownerSet`. That is, remove it and return it.
// ASSUMES: `ownerSet` is not empty
std::shared_ptr<Task> extract(std::unordered_set<std::shared_ptr<Task>>& ownerSet)
{
    auto begin = ownerSet.begin();
    std::shared_ptr<Task> first{*begin};
    ownerSet.erase(begin);
    return first;
}

TEST_CASE("Removal from the container should not cause undefined behavior") {

    std::unordered_set<std::shared_ptr<Task>> ownerSet;

    for (int i = 0; i < 100; ++i)
        ownerSet.emplace(std::make_shared<Task>());

    while (!ownerSet.empty()) {
        // The temporary returned by extract() will live until the semicolon,
        // so it will (barely) outlive the call to DoWork().
        extract(ownerSet)->DoWork();
        // This is equivalent to:
        //auto todo{extract(ownerSet)};
        //todo->DoWork();
    }

}

С одной точки зрения, это почти тривиальное изменение по сравнению с вашим подходом, так как все, что я сделал, это сдвиг ответственность от объекта задачи перед владельцем контейнера. Но с этим сдвигом беспорядок исчезает. Те же шаги выполняются, но они имеют смысл и почти вынуждены, когда перемещаются в более подходящий контекст. Чистый дизайн ведет к чистой реализации.