C++ successive checks of object variable value yield different results?

Question

I am new to C++, though I have worked with C and Java before.

In the following code, I:

Define the polygon class. For now it only has 1 variable: numpoints
create a global pointer to a polygon object that's null

define a handler for a click event, which if the object exists, just prints the value of numpoints. If it doesn't, it creates it and sets the value of numpoints to be 0.

//defining polygon class
class polygon{
public:
    int numpoints;  
};

//create a global pointer that's uninitialized
static polygon *current = NULL;

//define a click handler.
void leftClick(int x, int y){
    if (current==NULL){
        polygon newpoly;
        current = &newpoly;
        current->numpoints = 0;
        printf("created new polygon\n");
        printf("%i points\n", (*current).numpoints);

    }else{

        printf("polygon exists\n");
        printf("%i points\n", (*current).numpoints);
    }
}

After the first click, the program prints

    created new polygon
    0 points

as expected. However, after the second and subsequent clicks, it prints

    polygon exists
    -1567658064 points

Or some other seemingly random number. Anybody know what is going on here? Why is the value not staying at 0? Any help is appreciated.

score 0 · Accepted Answer · answered Feb 23 '17 at 12:31

This should work:

//defining polygon class
class polygon{
public:
    int numpoints;  
};

//create a global pointer that's uninitialized
static polygon *current = NULL;
polygon newpoly;    
//define a click handler.
void leftClick(int x, int y){
    if (current==NULL){

        current = &newpoly;
        current->numpoints = 0;
        printf("created new polygon\n");
        printf("%i points\n", (*current).numpoints);

    }else{

        printf("polygon exists\n");
        printf("%i points\n", (*current).numpoints);
    }
}

The problem is that newpoly is destroyed after the first printf because it goes out of scope. You have to learn how memory is managed in C++.

Telokis · Answer 2 · 2017-02-23T12:47:42.433

0

newpoly is a local variable. You are taking its address but it is destroyed right after so that address doesn't make sens anymore.

What you could do is use dynamic allocation instead : current = new polygon;.

But it is generally bad to use dynamic allocation without wrapping it in some way.

If you are using C++11, you can use a std::unique_ptr<polygon> from the header <memory>.

The result is

static std::unique_ptr<polygon> current; // No need to set it to NULL
...
current.reset(new polygon);

This changes will ensure that your allocation is properly deleted when needed.

edited Feb 23 '17 at 12:47

answered Feb 23 '17 at 12:31

Telokis

3,399
14
36

This is very bad advice. Creating floating and non-managed pointers is not the correct solution. – The Quantum Physicist Feb 23 '17 at 12:34
It's a working solution, even though it's not ideal – Telokis Feb 23 '17 at 12:40
I would be careful when giving advice to beginners. You should target giving good advice instead of just getting stuff that works. At least mention that this is a bad solution due to this and that, and a good solution is so and so. In your case, if you want to mention a good solution, recommend wrapping this in a class and get the destructor to take care of of deletion, or maybe use smart pointers inside the class. – The Quantum Physicist Feb 23 '17 at 12:41
You're right, I will edit my answer. At first, I thought that, since the OP is not very used to C++, I should keep it simple but I'll update. – Telokis Feb 23 '17 at 12:43

C++ successive checks of object variable value yield different results?

2 Answers2