Job/Task consuming queue, a valid case for self-erasing items?

Question

Self-deletion (or self-erasure from a container) is often regarded as a bad practice (for good reasons), but I am wondering if self-erasure is a legitimate approach for the following case:

We have a queue of tasks, a process consumes the queue (the queue.front() task).

• For some tasks (Guaranteed Tasks) the execution is guaranteed, as long as it is not completed, it remains at the front of the queue. Once successful it is removed, queue.pop().

• For some other tasks (Ephemeral Tasks) we do not care about completion, we attempt and we pop anyway if it fails or succeed.

This might be complete over-optimization but I do not like testing when I am reading the front task. Because we already know what the behavior should be when we are pushing the task in the queue. So switching/branching when emptying the queue appears to me as a design failure.

Here is a very simple, single file example that you can copy/paste and compile:

#include <iostream>
#include <queue>
#include <memory>

class TaskQueue;

class Task {
  public:
    virtual void consume(TaskQueue& queue) = 0;
};

using ITask = std::unique_ptr<Task>;

class TaskQueue {
  public:
    std::queue<ITask> tasks;
    void process() {
        while(!tasks.empty()) {
            tasks.front()->consume(*this);
        }
    }
};

class Ephemeral : public Task {
  public:
    explicit Ephemeral(std::string data) : data(std::move(data)) {};
    std::string data;
    void consume(TaskQueue& queue) override {
        std::cout << "Might fail but I am leaving anyway! " << data << std::endl; // Do some work that might fail
        queue.tasks.pop(); // SELF-ERASURE
    };
};

class Guaranteed : public Task {
  public:
    explicit Guaranteed(std::string data, unsigned int repetitions) : data(std::move(data)), repetitions(repetitions) {};
    std::string data;
    unsigned int repetitions; // For demonstration purpose
    unsigned int attempt_count;
    void consume(TaskQueue& queue) override {
        std::cout << "I am not leaving unless I succeed! " << data << std::endl;
        ++attempt_count;
        bool success = attempt(); // Might (artificially) fail
        if(success) { queue.tasks.pop(); } // SELF-ERASURE on success
    };
    bool attempt() { return attempt_count == repetitions;}; // Do some work that might fail
};

int main() {
    ITask task1 = std::make_unique<Ephemeral>("Fire and forget!");
    ITask task2 = std::make_unique<Guaranteed>("Success on first try!", 1);
    ITask task3 = std::make_unique<Guaranteed>("Give me some time!", 3);
    ITask task4 = std::make_unique<Ephemeral>("I did it!");
    ITask task5 = std::make_unique<Guaranteed>("Some troubles ahead!", 2);
    TaskQueue task_queue;
    task_queue.tasks.push(std::move(task1));
    task_queue.tasks.push(std::move(task2));
    task_queue.tasks.push(std::move(task3));
    task_queue.tasks.push(std::move(task4));
    task_queue.tasks.push(std::move(task5));
    task_queue.process();
}

Result:

Might fail but I am leaving anyway! Fire and forget!
I am not leaving unless I succeed! Success on first try!
I am not leaving unless I succeed! Give me some time!
I am not leaving unless I succeed! Give me some time!
I am not leaving unless I succeed! Give me some time!
Might fail but I am leaving anyway! I did it!
I am not leaving unless I succeed! Some troubles ahead!
I am not leaving unless I succeed! Some troubles ahead!

Do you consider this proper code or is there a better way? It seems over convoluted to me but I struggle to find a proper approach not using self-deletion/self-erasure and NOT testing again in the process() function.

Finally, I think we might reformulate this question this way: Is it OK to have a container where elements can leave on their own?

Something like this:

GatheringQueue<Participant> gathering_queue{element1, element2, element3};
Participant element = gathering_queue.get_front();
// Some stuff
element.leave(); // We leave the queue for some reason

In my mind it is kind of similar to a line in a Post Office, some people in the line can wait and see if their package actually leaves, some other will just leave the package here and don't care about what happens, they leave the line immediately.

For the sake of completeness here is all I could find on stack overflow, more or less related to the topic:

Object delete itself from container

Remove self from the container in the lambda

Self erasing workers c++

Answer 1

Why not just have an ephemeral flag on each task and do the popping in TaskQueue:

#include <iostream>
#include <queue>
#include <memory>

class TaskQueue;

class Task {
  public:
    Task(bool ephemeral): ephemeral(ephemeral) {}
    virtual ~Task() {}
    virtual bool process() = 0;

    bool isEphemeral() const { return ephemeral; }
  private:
    const bool ephemeral;
};

using ITask = std::unique_ptr<Task>;

class TaskQueue {
  public:
    std::queue<ITask> tasks;
    void process() {
        while(!tasks.empty()) {
            auto& task = tasks.front();
            bool success = false;
            try
            {
                success = task->process();
            }
            catch (std::exception& ex)
            {
                std::cout << "task failed: " << ex.what();
            }
            if (success || task->isEphemeral())
            {
                tasks.pop();
            }
        }
    }
};

class Ephemeral : public Task {
  public:
    explicit Ephemeral(std::string data) : Task(true), data(std::move(data)) {};
    std::string data;
    bool process() override {
        std::cout << "Might fail but I am leaving anyway! " << data << std::endl; // Do some work that might fail
        return true;
    };
};

class Guaranteed : public Task {
  public:
    explicit Guaranteed(std::string data, unsigned int repetitions) : Task(false), data(std::move(data)), repetitions(repetitions) {};
    std::string data;
    unsigned int repetitions; // For demonstration purpose
    unsigned int attempt_count;
    bool process() override {
        std::cout << "I am not leaving unless I succeed! " << data << std::endl;
        ++attempt_count;
        bool success = attempt(); // Might (artificially) fail
        return success;
    };
    bool attempt() { return attempt_count == repetitions;}; // Do some work that might fail
};

int main() {
    ITask task1 = std::make_unique<Ephemeral>("Fire and forget!");
    ITask task2 = std::make_unique<Guaranteed>("Success on first try!", 1);
    ITask task3 = std::make_unique<Guaranteed>("Give me some time!", 3);
    ITask task4 = std::make_unique<Ephemeral>("I did it!");
    ITask task5 = std::make_unique<Guaranteed>("Some troubles ahead!", 2);
    TaskQueue task_queue;
    task_queue.tasks.push(std::move(task1));
    task_queue.tasks.push(std::move(task2));
    task_queue.tasks.push(std::move(task3));
    task_queue.tasks.push(std::move(task4));
    task_queue.tasks.push(std::move(task5));
    task_queue.process();
}

The tasks are now self contained and don't require knowledge of their container, more importantly its removed the undefined behaviour of objects destroying themselves.

You could remove the ephemeral flag and just make isEphemeral virtual if you prefer.

I've also added a virtual destructor to Task otherwise derived classes won't be deleted properly. I've added exception handling to the task processing just in case but this can be removed if your tasks don't throw exceptions.