0

I have an application which contains instances of an class Obj with each instance identified by an ID integer as a class members. The objects are arranged as a top level object with branches of nested sub-objects stored in a std::vector like this:

/* Obj tree with L ID as:
 *
 *      Obj (L 0)
 *          |
 *      Obj (L 1)
 *          |
 *      Obj (L 2)
 *
 */

I have defined a utility method to redirect a pointer to an object in the hierarchy that has a given ID. This is implemented as a recursive call to the utility from each object with sub-objects until the object is found or the bottom of the hierarchy has been reached. The recursion should then unroll and return the pointer to the application scope. For this reason I have passed the pointer as a reference so it may be changed within the appropriate recursive call.

Source code is as follows:

The Object class is

// Object class
class Obj {

    friend class Utils;
    friend class App;

public : Obj(int ID_L) // Overloaded constructor
    {
        this->ID_L = ID_L;
    }

    // Public and private IDs
    public : int ID_L;  

    // Vector of sub objects
    std::vector<Obj> subObj;

    // Add sub-obj method
    public : void addSubObj(int num_layers) {

        // Add the sub-obj
        subObj.emplace_back( this->ID_L + 1);

        // Add one underneath if necessary
        if (subObj.back().ID_L <= num_layers) {
            subObj.back().addSubObj(num_layers - 1);
        }

    };

};

The Utility class is

// Utility class
class Utils {

// Static method to return pointer to obj in the hierarchy
public : static void getObj(Obj*& ObjTree, int ID_L, Obj*& ObjRet) {

    // Try match on the top of the tree given
    if (ObjTree->ID_L == ID_L) {
        ObjRet = ObjTree;   // Match found so update pointer and return     
        return;

    } else {

        // Loop through array of subObjs on this Obj
        for (Obj o : ObjTree->subObj) {

            // Create pointer to object in present scope
            Obj* O = &o;

            // Look for match on this subObj
            Utils::getObj(O, ID_L, ObjRet); // Recursvie call passing reference of pointer along and address of present sub-Obj and its children

                // If a match found then return the non-null pointer
                if (ObjRet != NULL) {
                    return;
                }

            }

        }

        // Specified Obj has not been found in the tree
        return;

    }

};

The application class is

// Application class
class App {

public : void run () {

    // Create object tree as above
    Obj TopObj(0);          // Top level obj
    TopObj.addSubObj(2);        // Left side branch

    // Create pointer to Obj to do further stuff with
    Obj* _o;

   // Create pointer to tree
   Obj* Tree = &TopObj;

    // Loop over all desired ID combos and return pointer to appropriate Obj in hierarchy
    for (int l = 0; l <= 2; l++) {

            // Null the pointer in preparation for next Obj
            _o = NULL;

            // Try to fetch a particular Obj from ObjTree
            Utils::getObj(Tree, l, _o);

            // If pointer is not null then Obj has been found so try to read IDs from it
            if (_o != NULL) {
                std::cout << "FOUND -- L" << l << " : reading ID from pointer as L" << _o->ID_L << " : " << _o << std::endl;
        }

    }

}

};

And the entry point is here:

// Entry point
int main(int argc, char* argv[]) {

    // Start application
    App a;
    a.run();
}

When debugging I have written out the value of the pointer _o to the screen and I can see it change when the object is found and then maintain its value as the recursion unrolls. However When trying to access the object to which it points to print out the ID to the screen in application scope the numbers are junk. This is the case when the object has been found 2 layers deep into the hierarchy or more. It does work when only drilling down one level.

Since I passed the pointers as references I was confident that they will be carried through scope but that doesn't seem to be the case as it seems like the object is going out of scope.

Can anyone see the problem with this implementation and perhaps suggest an improved implementation for such an object retrieval utility?

6
  • 4
    Is all that code needed to showcase the issue? Could you post a minimal reproducible example (the keyword being minimal)? Commented Jan 21, 2016 at 10:38
  • 1
    [OT]: Obj(){}; lets member uninitialized, should probably be removed. Commented Jan 21, 2016 at 10:39
  • [OT]: Better signature for Obj* is static Obj* getObj(Obj& ObjTree, int ID_L, int ID_R) Commented Jan 21, 2016 at 10:42
  • A java flavored C++. Commented Jan 21, 2016 at 10:43
  • Obj* O = &o; creates a pointer to a local variable and it becomes invalid as soon as the current loop iteration ends. Commented Jan 21, 2016 at 10:46

1 Answer 1

2

In

for (Obj o : ObjTree->subObj) {
    Utils::getObj(&o, ID_L, ID_R, ObjRet);

You make a copy of Obj and ObjRet may have dangling pointer once leaving scope.

use instead:

for (Obj& o : ObjTree->subObj) {
    Utils::getObj(&o, ID_L, ID_R, ObjRet);
Sign up to request clarification or add additional context in comments.

1 Comment

Thank you Jarod for your advice and fast answer.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.