C++ Debug Assignment - Reading Linked List

[venros]

Hi guys,

This is a debug assignment for a linked list, I edited many things but the COUT is still broken. It couts the first value entered but thats it.

#include<iostream>

using namespace std;


struct Node
{
  int val;
  Node *p_next;
};

int main()
{
  int val;
  Node *p_head = new Node;
  Node *p_curr = new Node;
  p_head = p_curr;

  while ( 1 )
  {
    cout << "Enter a value, 0 to replay: " <<endl;
    cin >> val;
    if ( val == 0 )
    {
      break;
    }
    Node *p_temp = new Node;
    p_curr->val = val; 
    p_curr->p_next = p_temp;
    //p_temp->p_next = NULL;
    p_curr = p_temp;
  }
  Node *p_itr = p_head;
  while ( p_itr != NULL )
  {
    cout << p_itr->val << endl;
    p_itr = p_itr->p_next;
    delete p_itr;
  }
  system("pause");
}

Ty

[ne555]

line 18 p_head = p_curr; memory leak
line 35 while ( p_itr != NULL ) given that line 31 is commented out, this condition makes no sense
line 39 delete p_itr;next iteration you'll try to dereference p_itr, but you just delete it

[venros]

Dumb mistakes how could I have missed that >.>

Atm that fixed all my issues except it still goes one extra step, so if i inputed:

1,2,3,4

output is:

1,2,3,4,0

Dont want it to cout that final val if its NULL.

    p_curr->val = val; 
    p_curr->p_next = p_temp;
    p_temp->p_next = NULL;
    p_temp->val = NULL;
    p_curr = p_temp;

Also not sure how to fix the mem leak, ty.

[ne555]

Upload your updated code so we can see your fixes

About the memory leak,

  Node *p_head = new Node; //¿what is the purpose of this line?
  Node *p_curr = new Node;
  p_head = p_curr;


  Node *p_curr = new Node;
  Node *p_head = p_curr;

[venros]

struct Node
{
  int val;
  Node *p_next;
};

int main()
{
  int val;

  Node *p_curr = new Node;
  Node *p_head = p_curr;


  while ( 1 )
  {
    cout << "Enter a value, 0 to replay: " <<endl;
    cin >> val;
    if ( val == 0 )
    {
      break;
    }
    Node *p_temp = new Node;
    p_curr->val = val; 
    p_curr->p_next = p_temp;
    p_temp->p_next = NULL;
    p_temp->val = NULL;
    p_curr = p_temp;
  }
  Node *p_itr = p_head;
  while ( p_itr != NULL )
  {
    cout << p_itr->val << endl;
    p_itr = p_itr->p_next;
  
  }
  
  delete p_itr;
  system("pause");
}

[ne555]

At line 13 you've got one node, but the user has not inputted anything yet. That node is uninitialized.
Later in line 23, after the user input, you create another node. So you've got two nodes but only one valid value.

The last node that has valid data is pointing to a node that points to NULL. You've got one extra superfluous node.


line 38: delete p_itr; ¿what about all the others nodes that were allocated? ¿would you not delete them?

[venros]

I created P_Temp after the user input to move over to P_curr ->next, how else do you recommend I do that with just one node P_Curr.

[ne555]

using constructors to facilitate creation

struct Node
{
  int val;
  Node *p_next;
  Node(): val(0), p_next(NULL){}
  Node(int val): val(val), p_next(NULL){}
};

1) empty header

//head.val is invalid, the first value is at head.next->val
Node head; //note that is an object, not a pointer
Node *top = &head;
while( std::cin>>val and val not_eq 0 ){
   top->next = new Node(val); //push_back
   top = top->next;
}

//traversal
Node *it = &head;
while( it->next ){
   std::cout << it->next->val < '\n';
   it = it->next;
}

a) especial case for first insertion

Node *head = NULL; //now it is a pointer
Node *top = head;
while( std::cin>>val and val not_eq 0 ){
   if( top == NULL )
      top = head = new Node(val);
   else{
      top->next = new Node(val); //push_back
      top = top->next;
   }
}

//traversal
Node *it = &head;
while( it ){
   std::cout << it->val < '\n';
   it = it->next;
}

you should encapsulate this in a 'list' class

[venros]

This is simply a debugging HW, I have to fix it as is. I know how to make a linked list via class, the issue here is I have to understand why and how to fix the simple program.

I appreciate the time you took to write that up.

[QLong]

#include<iostream>
using namespace std;
struct Node
{
  int val;
  Node *p_next;
  Node(int v) {val=v;p_next=NULL;}
};

int main()
{
  int val;
  Node *p_head;
  Node *p_curr=new Node(0);
  p_head = p_curr;

  while ( 1 )
  {
    cout << "Enter a value, 0 to replay: " <<endl;
    cin >> val;
    if ( val == 0 )
    {
      break;
    }
    Node *p_temp = new Node(val);
    //p_temp->val = val; 
    p_curr->p_next = p_temp;
    //p_temp->p_next = NULL;
    //p_curr = p_temp;
    p_curr=p_temp;
  }
  //p_head=p_curr->p_next;
  Node *p_itr = p_head->p_next;
  while ( p_itr != NULL )
  {
	Node *p=p_itr->p_next;
	cout << p_itr->val << endl;
        delete p_itr;
	p_itr=p;
  }
  system("pause");
}

some little problems in connecting and deleting the nodes...

[venros]

works great ty Qlong & ne555 learned something new with your constructors, and why it was doing what it was.

You can mark this solved.