0

Hey there I am new to Object oriented programming in C++ (coded in Java) and I have some questions regarding it.

I have a two methods

void Manager::generate(){
    // Generate robot basis
    Matrix4 robotBasis = generateRobotBasis();

    // Create Human 
    LinkedList * ll = new LinkedList();

    createLL(ll);

    Node *j = ll->GetRoot();
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
    ...
}

void Manager::createLL(LinkedList * ll){
    Node node();
    ll->InsertRoot(&node);
    Node node2();
    ll->InsertChild(&node, &node2);
    Node node3();
    ll->InsertChild(&node2, &node3);

    Node *j = &node;
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
    j = ll->GetRoot();
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
}

Results:

00ABF540

00ABF498

00ABF540

00ABF498

00ABF540

00CB83A8

The last column should be 00ABF498 ? But turns out to be some random memory address.

Holt
  • 36,600
  • 7
  • 92
  • 139
  • 8
    This is a function declaration: `Node node();`. use `Node node;`. – juanchopanza May 21 '14 at 14:47
  • 2
    Hopefully being a Java programmer, you realize that doing this is not necessary in C++: `LinkedList * ll = new LinkedList();`. Java is not C++, as all that does is create a memory leak if not handled correctly. All you need to do is this: `LinkedList ll;` and then just pass the address to those functions you're calling. – PaulMcKenzie May 21 '14 at 14:56
  • Can you show the LinkedList code? Your statement of being a java programmer and the fact that you insert a local stack variable as pointer could mean that you just store a pointer to its address instead of copying it – MatthiasB May 21 '14 at 14:58
  • @djikay It definitely is a function declaration. – PaulMcKenzie May 21 '14 at 14:59
  • 1
    @Griwes That is [highly debatable](http://stackoverflow.com/questions/14589346/) :) Some say C++ is context-free, some say it is context-sensitive, and some say [it is even worse](http://stackoverflow.com/a/14589567/252000). – fredoverflow May 21 '14 at 15:00
  • We need to see the code of "LinkedList::insertChild" and "LinkedList::insertRoot". – Daniel Heilper May 21 '14 at 15:04
  • Additionally to PaulMcKenzie answer, either you do `LinkedList ll; createLL(&ll)` or `LinkedList ll; createLL(ll)` and define `void Manager::createLL(LinkedList& ll){...`. That is, pass by reference instead of pointer. This is similar to what happens in Java. Cheers – cassinaj May 21 '14 at 15:08

2 Answers2

0

We don't have the LinkedList insert methods code, so it's only a hunch about the source of the problem

Assuming insertRoot and insertChild don't allocate a node object, the "Node" objects in your code ends their life at the end of the Manager::createLL's scope (they are allocated on the stack). So just allocate them on the heap: You may try this:

void Manager::createLL(LinkedList * ll){
Node* node = new node ();
ll->InsertRoot(node);
Node* node2 = new Node ();
ll->InsertChild(node, node2);
Node* node3 = new node ();
ll->InsertChild(node2, node3);

Node *j = node;
std::cout << j << "\n";
j = j->pChild_;
std::cout << j << "\n";
j = ll->GetRoot();
std::cout << j << "\n";
j = j->pChild_;
std::cout << j << "\n";

}

The reason you are able to see the root's address, isn't because insertRoot is working. You just copied the address, but when you get out of the "CreateLL" The node of the root is also deallocated, and the address refers to some unrelated memory.

Also, take care of deallocation of the nodes object memory to avoid memory leaks (current C++ standard has no built-in garbage collection).

Daniel Heilper
  • 1,182
  • 2
  • 17
  • 34
0

If you remove the ()'s to make the code actually compile, you'll be storing stale pointers in your list (unless you copy the nodes into the tree rather than storing the pointers).

These point at the destroyed local variables from the call to createLL.
Dereferencing any of them is undefined, and you'll most likely get garbage values or crashes if you try.

If you want an object to "outlive" a function call that creates it, you must use new.

It should be more like this (untested):

void Manager::generate(){
    // Generate robot basis
    Matrix4 robotBasis = generateRobotBasis();

    // Create Human 
    LinkedList ll;

    createLL(ll);

    Node *j = ll.GetRoot();
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
    ...
}

void Manager::createLL(LinkedList & ll){
    Node* node = new Node;
    ll.InsertRoot(node);
    Node* node2 = new Node;
    ll.InsertChild(node, node2);
    Node* node3 = new Node;
    ll.InsertChild(node2, node3);

    Node *j = node;
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
    j = ll->GetRoot();
    std::cout << j << "\n";
    j = j->pChild_;
    std::cout << j << "\n";
}

Java and C++ are vastly different - you'll need to unlearn a lot of Java.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82