class Element {
public:
    ElementTypes type = DOT;

    Element() {}
    Element(ElementTypes type) : type(type) {}

    virtual void Draw() { return; }
};

class Dot : public Element {
public:
    int x, y;

    Dot(int x, int y) : x(x), y(y) {}

    void Draw() override {
        DrawCircle(x, y, 2.f, BLACK);
    }
};

class Drawing {
public:
    std::vector<Element*> Elements;

    void AddDot(Dot& dot) {
        Elements.emplace_back(&dot);
    }

    void Draw() { 
        for (auto element : Elements) {
            element->Draw();
        }
    }
};

For some reason, there is a crash when trying to call element->Draw().

Exception thrown at 0x00007FF66DDC1486 in geometry.exe: 0xC0000005: Access violation reading location 0x0000000000000000.

I am using the function AddDot to add an element to the vector

Not using a pointer to the class, the Draw function is just not overriden.

CodePudding user response：

Welcome to your first Access Violation!
Do not panic. :)

Access violation reading location 0x0000000000000000

Access Violation a location like 0x0000 (zeros) are likely just a nullptr. Since you hold something like std::vector<Element*> Elements; it may be understandable where it comes from.

To add a new Element you'll need to actually allocate.
The usage of Dot& dot passes it by reference to an unknown location which isn't a good practice since you'll hit UB pretty fast (Or worse, heap corruptions).

To allocate a new element you'll need to do:

std::vector<Element*> Elements;

Elements.push_back(new Element(/*Params here*/));

Since you don't have actually any expensive values in your class you may hold it by value instead:

std::vector<Element> Elements

For the sake of simplicity do not go into why it should be value or pointer.

The virtual functions do not play a role here since Drawing doesn't inherit Element.
I'd suggest making something like Drawable So any entity that needs to draw may inherit it.

Best of luck!

CodePudding user response：

The problem is indeed probably that dot, which address was added to the vector, no longer exists when you do the drawing. A typical case is if you initialise the Drawing in a function, using local objects that get destroyed when leaving the initialization function.

One way of solving it is to ensure that all the figures that are added in the Drawing are created on the freestone with new. You'd still have to take care of their deletion later on. This is error prone.

A better option would be to go for smart pointers instead of raw pointers. This makes sure that the object is kept alive as long as there are some smart pointers pointing to it, and that th object is destroyed once it's no longer used (hint: work on Drawing's destructor. Here the change:

class Drawing {
public:
    std::vector<shared_ptr<Element>> Elements;

    void AddDot(shared_ptr<Element> dot) {
        Elements.emplace_back(dot);
    }

    void Draw() { 
        for (auto element : Elements) {
            element->Draw();
        }
    }
};
void init (Drawing&d) {
    auto dt1 = make_shared<Dot>(10,30); 
    d.AddDot(dt1);
}
int main() {
    Drawing d; 
    init (d);
    d.Draw(); 
}

(Online demo)

The advantage is that you can safely transform AddDot() into a polymorphic AddElement() and avoid creating a new add function for every possible shape.

Another approach is not as sophisticated and robust as the smart pointer, but relatively easy to implement: take the dot that is passed as argument and make a copy that is owned (and later destroyed) by the Drawing. Unfortunately, with your current code, it would be cumbersome as you'd need a different function for every subclass of Element that you would like to add (because their constructor may take different arguments and you don't want object slicing to happen). So you'd need to give Element a clone() function that returns a pointer to a newly created Element of the same type (this is the prototype pattern).