CODEWITHSUNDEEP
c++
Mark
Mark

Reputation: 8688

C++ function using reference as param error

struct node {

  string info;
  node *next;

  node() {
   info = "string";
   next = NULL;

  }

};

void insert(node &anode) {
 node newNode;
 anode.next = &newNode;
}

What is wrong with this implementation of insert for this structure ? How should I fix this? added: Very sorry that I wasn't clear. I know what is wrong with the program. I want to know how I can insert a new node to a reference node. Since I am using a reference to a node as a param this mean that node must not be a pointer? So its stored on stack? which means I can't use memory from heap? (or else seg fault?) so how am I suppose to use new ? This is my main confusion. Perhaps my approach is wrong but I don't see why it should be.

Upvotes: 0

Views: 164

Answers (7)

Christian Hresko
Christian Hresko

Reputation: 71

If you insist on using free functions, your best bet is probably something like:

static node* head = NULL;
static node* current = NULL;

void insert(std::string& val)
{
    if (!head) {
       current = new node(val);
       head = current;
    } else {
       current->next = new node(val);
       current = current->next;
    }
}

and having your constructor accept an std::string as an argument. Relying on an entity outside the function to create nodes for you is probably not the best idea. You can pseudo-encapsulate that by creating nodes on demand when you call insert. Then you can run through the nodes using the head pointer and consequently delete them when you're finished with the list.

Upvotes: 1

juanchopanza
juanchopanza

Reputation: 227558

What's wrong is that newNode lives in the scope of the insert function. You probably want something like

void insert(node &anode) {
 node* newNode = new node;
 anode.next = newNode;
}

but the parent node, or something else, then has to take care of the new node's lifetime. It now owns the next node. If you want the caller to be in charge, then this might be more suitable:

void insert(node& parentNode, node& nextNode) {
 parentNode.next = &nextNode;
}

Note that you can avoid some of the lifetime issues by using boost::shared_ptr or std::shared_ptr if you have access to C++0x. These smart pointers basically wrap a pointer and take care to delete it when nobody is using it. The code would look something like this:

struct node {
  // other data members...

  shared_ptr<node> next;

  // constructors/destructors 
};

void insert(node& anode) {
  anode.next = shared_ptr<node>(new node);
}

Now you don't have to worry about deleting the new node at any point.

Upvotes: 5

Ozair Kafray
Ozair Kafray

Reputation: 13549

Assuming that you are talking about a runtime error. The problem is that in your insert function 

node newNode

is only a local variable, and it will be causing a problem when you try to access it later while iterating on the node(s).

Inside the insert function you should be doing something like this:

node* newNode = new node();
anode.next = newNode;

Upvotes: 1

Dominic Gurto
Dominic Gurto

Reputation: 4205

You are using a static address.

void insert(node &anode) {
 node newNode;
 anode.next = &newNode;
}

newNode is a local object. At the end of the function, it will go out of scope and its address will be invalid.

Upvotes: 0

Xeo
Xeo

Reputation: 131877

The problem is that the local variable newNode will go out of scope once the function insert exists, and anode.next will now reference an invalid node.

Upvotes: 1

AnT stands with Russia
AnT stands with Russia

Reputation: 320729

The "what's wrong" has nothing to do with references.

This implementation stores a pointer to a local variable in anode.next. Local variable gets destroyed immediately afterwards (when insert function exists), while the pointer continues to live pointing into a destroyed location.

Upvotes: 1

MartinStettner
MartinStettner

Reputation: 29174

You're returning (implicitly, as member of anode) a pointer to the local variable newNode. newNode is destroyed when you're leaving insert, so anode.next contains an invalid pointer afterwards.

BTW: should this question be tagged "homework"? :)

Upvotes: 2

Related Questions