3

I'm having trouble trying to call the override function of an object that was appended to a vector. I'm not quite sure if I just don't completely understand pointers and references, so I'm not quite sure what to look for when debugging this issue.

Here is my code:

#include <iostream>
#include <vector>

class Task {
    public:
        Task(std::vector<Task> *list) {
            list->push_back(*this);
        }

        virtual void Run() {
            std::cout << "Called Task Run!" << std::endl;
        }
};

class OverrideTask : public Task {
    public:
        OverrideTask(std::vector<Task> *list) : Task(list) {}
        void Run() override {
            std::cout << "Called Override Run!" << std::endl;
        }
};

int main() {
    std::cout << "Main method entered" << std::endl;
    std::vector<Task> listOfTasks;

    OverrideTask ot = OverrideTask(&listOfTasks);
    Task t = Task(&listOfTasks);

    for(int i = 0; i < listOfTasks.size(); i++) {
        listOfTasks[i].Run(); // Will print "Called Task Run!" twice.
    }

    ot.Run(); // Prints "Called Override Run!"
    t.Run(); // Prints "Called Task Run!"
}

When I loop through the vectors, it seems that I can't call the override function, but when I call them directly from the object, they seem to work. Can anybody point me in the correct direction?

liver man
  • 33
  • 3
  • 5
    You have fallen victim to [slicing](https://stackoverflow.com/questions/274626/what-is-object-slicing). I.e. `vector.push_back(OverrideTask{})` will strip the overridden part and preserve only its `Task` part. Other than that the code is OK, just use `std::vector>` and it will work. – Quimby Mar 14 '21 at 09:28
  • You also have fallen victim of misusing constructors for side effects. You never can be sure if a constructor is called for a temporary object, a local object or a more lasting object. So a better design would really to focus constructor on construction, and move the code for using the new object (i.e. responsibility for adding a chosen task to a task list) to another place. – Christophe Mar 14 '21 at 10:11
  • cppcheck also suggested using explicit for a constructor with one argument. – Allan Wind Mar 14 '21 at 10:13

1 Answers1

3

Try store the base pointer in the vector instead like this. @Christophe asked me to remind that we store pointers which means the Task object must remain alive. See also also @Quimby's note about using unique_ptr instead of Task *.

#include <iostream>
#include <vector>

class Task {
    public:
        Task(std::vector<Task *> *list) {
            list->push_back(this);
        }

        virtual void Run() {
            std::cout << "Called Task Run!" << std::endl;
        }
};

class OverrideTask : public Task {
    public:
        OverrideTask(std::vector<Task *> *list) : Task(list) {}
        void Run() override {
            std::cout << "Called Override Run!" << std::endl;
        }
};

int main() {
    std::vector<Task *> listOfTasks;
    OverrideTask ot = OverrideTask(&listOfTasks);
    Task t = Task(&listOfTasks);
    for(int i = 0; i < listOfTasks.size(); i++) {
        listOfTasks[i]->Run();
    }
} 
Allan Wind
  • 23,068
  • 5
  • 28
  • 38
  • This seems to work! I guess I really do need to read more up on pointers, and more specifically this "base pointers" concept. Thank you! – liver man Mar 14 '21 at 09:43
  • give upvote would much appreciated – Ashish Kamble Mar 14 '21 at 09:45
  • Upvote well deserved. However, you should warn OP that this solution require the original task object to be still alive when the task list is used. In the simple example it works, but OP used a copy by value principle, and we don't know if the intent was not to initialize a task list somewhere, and later invoke the task list. – Christophe Mar 14 '21 at 10:08
  • Good point. Hopefully my write-up does your comment justice. – Allan Wind Mar 14 '21 at 10:11
  • Perfect. And for the records here an alternative code using unique_ptr and prototype pattern to clone tasks added to the list. But the design of populating the list in the constructor doesn't help using virtual functions ;-) https://ideone.com/GXZ1jX – Christophe Mar 14 '21 at 10:14