Fabrizio
Fabrizio

Reputation: 1074

Recursive Class Definition yields to invalid pointers in c++

I'm trying to create a class that has a recursive structure, like a Matrioska doll, to be clearer. Like this A class contains 8 children of type A. I'm using pointers as pointed out in this question: Can a c++ class include itself as an member?

And I'm assigning an Index value at each class instance, to keep track of how down the class is with respect to the parent one.

This is My code:

Tester.h

#pragma once

const unsigned int MAX_RECURSION = 3;

struct Tester
{

    Tester* Children[2][2][2];
    unsigned int Index;

    Tester(unsigned int index) : Index(index), Children()
    {
        std::cout << std::string(index, '-') << "(" << index << ")" << std::endl;
        if (index < MAX_RECURSION) {
            for (int x = 0; x < 2; x++) {
                for (int y = 0; y < 2; y++) {
                    for (int z = 0; z < 2; z++) {

                        this->Children[x][y][z] = &Tester(index + 1);;
                    }
                }
            }
        }

    }

    void Walk(int& tr)
    {

        if (this->Index < MAX_RECURSION) {
            for (int x = 0; x < 2; x++) {
                for (int y = 0; y < 2; y++) {
                    for (int z = 0; z < 2; z++) {
                        tr++;
                        (this->Children[x][y][z])->Walk(tr);
                    }
                }
            }

        }
    }

};

Main.cpp:

#include "Tester.h"
#include <iostream>
int main() {

    int c = 0;

    Tester t(1);
    t.Walk(c);

    return 0;
}

The value c holds the number of times the function Tester::Walk run.

By running this code, all of the classes get correctly created, as you can be seen in the output log:

-(1)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
--(2)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)
---(3)

A class with index 3 was indeed created 8*8 times so 64. (8 for each recursion).

But when I try to Walk down the Children variable of the t, the walk function only gets run 8 times. By debugging I later found out that only the first 8 Tester classes (the first inside the t clas in main.cpp) have the right index, as the other ones have an index of 3435973836. I initially thought that the recursive creation had problems, but the log shows that the indexes work correctly (the one gets printed 1 time, the 2 8 times and the 3 64 times).

What is causing this behavior?

Upvotes: 1

Views: 78

Answers (1)

user4581301
user4581301

Reputation: 33952

Thou shalt not store pointers to temporary objects.

Their Lifetime is too short. The compiler may be warning you of this, if not preventing it outright with a compiler error. Eg. g++9.3 emits

..\src\test.cpp:18:68: error: taking address of rvalue [-fpermissive]
   18 |                         this->Children[x][y][z] = &Tester(index + 1);;
      |                                                                    ^

In

this->Children[x][y][z] = &Tester(index + 1);;

Tester(index + 1) creates a an unnamed, temporary object that only lives until the end of the line. Before you'll get a chance to use a pointer to this object, it has gone out of scope and been destroyed.

The most direct solution to this problem is to manually dynamically allocate

this->Children[x][y][z] = new Tester(index + 1);

but you must now manually manage the lifetime of all of the allocated Testers, and this can be a lot trickier than it appears.

The recommended solution would be to use a smart pointer to manage the lifetime of the Testers. I will use a std::unique_ptr because it is the simplest and most restrictive smart pointer:

#include <iostream>
#include <memory> // unique_ptr, make_unique
const unsigned int MAX_RECURSION = 3;

struct Tester
{

    std::unique_ptr<Tester> Children[2][2][2]; 
        // array of Testers with scope-managed lifetime. When the array goes out of 
        // scope, all of the Testers will be automatically destroyed.
    unsigned int Index;

    Tester(unsigned int index) :  Children(), Index(index)
    {
        std::cout << std::string(index, '-') << "(" << index << ")" << std::endl;
        if (index < MAX_RECURSION) {
            for (int x = 0; x < 2; x++) {
                for (int y = 0; y < 2; y++) {
                    for (int z = 0; z < 2; z++) {

                        this->Children[x][y][z] = std::make_unique<Tester>(index + 1);
                            // makes a unique_ptr referencing a brand-new Tester
                    }
                }
            }
        }

    }

    void Walk(int& tr)
    {

        if (this->Index < MAX_RECURSION) {
            for (int x = 0; x < 2; x++) {
                for (int y = 0; y < 2; y++) {
                    for (int z = 0; z < 2; z++) {
                        tr++;
                        (this->Children[x][y][z])->Walk(tr);
                    }
                }
            }

        }
    }
};

Upvotes: 2

Related Questions