0

I have a list of derived class objects of 3 types, each contains string and two integers.

#include "pch.h"
#include <iostream>
#include <list>
#include <string>
#include <algorithm>
#include <iterator>

class Shape
{
private:
    int x, y;
public:
    Shape() {}
    Shape(int &x_, int &y_) : x(x_), y(y_) {}
    virtual void Draw() {
        std::cout << this->x << ", " << this->y << "}\n";
    }
};

class Circle : public Shape
{
private:
    std::string type;
public:
    Circle() {}
    Circle(int &x_, int &y_) : Shape(x_, y_) { this->type = "Circle"; }
    void Draw() {
        std::cout << this->type << ":   {";
        Shape::Draw();
    }
};

class Triangle : public Shape
{
private:
    std::string type;
public:
    Triangle() {}
    Triangle(int &x_, int &y_) : Shape(x_, y_) { this->type = "Triangle"; }
    void Draw() {
        std::cout << this->type << ":   {";
        Shape::Draw();
    }
};

class Square : public Shape
{
private:
    std::string type;
public:
    Square() {}
    Square(int &x_, int &y_) : Shape(x_, y_) { this->type = "Square"; }
    void Draw() {
        std::cout << this->type << ":   {";
        Shape::Draw();
    }
};

void FillWithShapes(int n, std::list<Shape*> &ls) {
    int x, y, type;
    Circle cir;
    Triangle tri;
    Square sq;
    for (int i = 0; i < 10; i++) {
        type = rand() % 3;
        x = rand() % 100;
        y = rand() % 100;
        if (type == 0) {
            cir = Circle(x, y);
            ls.push_back(&cir);
        }
        else if (type == 1) {
            tri = Triangle(x, y);
            ls.push_back(&tri);
        }
        else if (type == 2) {
            sq = Square(x, y);
            ls.push_back(&sq);
        }
    }
}

int main()
{
    std::list<Shape*> shapes;
    FillWithShapes(10, shapes);
    std::for_each(shapes.begin(), shapes.end(), [](Shape *s) { s->Draw(); });
}

I'm getting read access violation exception when accessing list elements in lambda:

Exception thrown: read access violation.
s->**** was 0xCCCCCCCC.

But when I put the code from function FillWithShapes straight to main(), it works just fine:

Square:   {18, 95}
Triangle:   {82, 21}
Circle:   {2, 53}
Square:   {18, 95}
Triangle:   {82, 21}
Square:   {18, 95}
Circle:   {2, 53}
Circle:   {2, 53}
Triangle:   {82, 21}
Square:   {18, 95}

I have started learning c++ not long ago, so I have no idea what may cause this exception in this case, though I'm probably missing something simple but significant here.


UPD: Fixed function to create pointers on heap:

void FillWithShapes(int n, std::list<Shape*> &ls) {
    int x, y, type;
    Circle *cir = new Circle();
    Triangle *tri = new Triangle();
    Square *sq = new Square();
    for (int i = 0; i < 10; i++) {
        type = rand() % 3;
        x = rand() % 100;
        y = rand() % 100;
        if (type == 0) {
            *cir = Circle(x, y);
            ls.push_back(cir);
        }
        else if (type == 1) {
            *tri = Triangle(x, y);
            ls.push_back(tri);
        }
        else if (type == 2) {
            *sq = Square(x, y);
            ls.push_back(sq);
        }
    }
}
  • You are putting addresses in your list "ls.push_back(&sq);" But those addresse are on the stack which are no more valid once the function return.The object which you are taken address are on the stack and are destroy when you return from the function – Gojita Apr 01 '19 at 11:12
  • You might also want to look at smart pointers. For example, you can replace `std::list` by `std::list>` and insert elemets by `ls.push_back(std::make_unique(x,y));` (in C++14). – Daniel Langr Apr 01 '19 at 11:23
  • Thank you @Wander3r and @DanielLangr! Both solutions worked for me, but the task originally required to use standart pointers. – Dr. Cringe Apr 01 '19 at 11:37

2 Answers2

2

You're filling the list with pointers to variables local to your FillWithShapes() function and stored on the stack. Their lifetime ends after the function returns - so it's reasonable that you get an access violation.

When you hoist the code up to main(), the lifetime of the local variables is now the lifetime of main(), i.e. throughout your program and past your last accesses through the list of shapes - so no violation.

You might want to read: What and where are the stack and heap?

einpoklum
  • 118,144
  • 57
  • 340
  • 684
2

In your function FillWithShapes, you are creating objects of type Circle, Sqaure etc and pushing those pointers into the vector. Once that function goes out of scope, these pointers are no longer valid.

You could create the objects on the heap and push those in the vector with the burden of de-allocating once done with the vector.

Wander3r
  • 1,801
  • 17
  • 27