Emms
asked on
need to fix codes... delete/free node and redisplay linked list codes
this program insert 10 integers to form a list. a fuction to delete node in middle of list is not working(??) or displaying properly once a node is freed. see codes below and please help.
#include <stdio.h>
#include <stdlib.h>
int del_node(int *, int);
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int main()
{
int arr[10], key, i;
for(i=0; i<10; i++)
{
printf("Enter an integer [%d]", i+1);
scanf("%d", &arr[i]);
}
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(arr, key);
getch();
return 0;
}
int del_node(int *arr, int key)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr, tempPtr;
for(i=0; i<10; i++)
{
newPtr = malloc(sizeof(listNodePtr)
newPtr->data = arr[i];
newPtr->nextPtr = NULL;
if(startPtr == NULL)
startPtr = newPtr;
else
currPtr = startPtr;
while(currPtr-> nextPtr != NULL)
currPtr = currPtr->nextPtr;
currPtr->nextPtr = newPtr;
}
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
while(currPtr != NULL) /*********** please check codes ******/
{
tempPtr = currPtr;
if(tempPtr->data == key) /***** key is the value to delete *****/
{
currPtr = currPtr->nextPtr;
free(tempPtr);
}
else
currPtr = currPtr->nextPtr; /********* end of delete middle node ****/
}
currPtr = startPtr;
while(currPtr != NULL)
{
printf("%d->", currPtr->data);
currPtr = currPtr-> nextPtr;
}
printf("NULL\n");
return 0;
}
#include <stdio.h>
#include <stdlib.h>
int del_node(int *, int);
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int main()
{
int arr[10], key, i;
for(i=0; i<10; i++)
{
printf("Enter an integer [%d]", i+1);
scanf("%d", &arr[i]);
}
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(arr, key);
getch();
return 0;
}
int del_node(int *arr, int key)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr, tempPtr;
for(i=0; i<10; i++)
{
newPtr = malloc(sizeof(listNodePtr)
newPtr->data = arr[i];
newPtr->nextPtr = NULL;
if(startPtr == NULL)
startPtr = newPtr;
else
currPtr = startPtr;
while(currPtr-> nextPtr != NULL)
currPtr = currPtr->nextPtr;
currPtr->nextPtr = newPtr;
}
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
while(currPtr != NULL) /*********** please check codes ******/
{
tempPtr = currPtr;
if(tempPtr->data == key) /***** key is the value to delete *****/
{
currPtr = currPtr->nextPtr;
free(tempPtr);
}
else
currPtr = currPtr->nextPtr; /********* end of delete middle node ****/
}
currPtr = startPtr;
while(currPtr != NULL)
{
printf("%d->", currPtr->data);
currPtr = currPtr-> nextPtr;
}
printf("NULL\n");
return 0;
}
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
ASKER
Thx...ssnkumar...
I had previous trouble passing startPtr into del_node()...
int del_node(listNodePtr *, int);
int main()
{
int key;
listNodePtr startPtr;
...
del_node(&startPtr, key);
...
}
int del_node(listNodePtr *startPtr, int key)
{
listNodePtr currPtr ...;
currPtr = startPtr;
...
}
In the meantime I am looking into your solution.
I had previous trouble passing startPtr into del_node()...
int del_node(listNodePtr *, int);
int main()
{
int key;
listNodePtr startPtr;
...
del_node(&startPtr, key);
...
}
int del_node(listNodePtr *startPtr, int key)
{
listNodePtr currPtr ...;
currPtr = startPtr;
...
}
In the meantime I am looking into your solution.
ASKER
Sorry, i mean to review sunnycoder's solution.
ASKER
I have reorganise my codings as follows for readibility and it doesn't work as I suppose is the passing of startPtr across functions. Also implemented sunnycoder's logic and has not been working too.
******************
#include <stdio.h>
#include <stdlib.h>
#include <alloc.h>
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int create_list(int *);
int add_node(int *);
int del_node(listNodePtr, int);
int display_list(listNodePtr);
int main()
{
int arr[10], key, i;
listNodePtr startPtr;
create_list(arr);
add_node(arr);
display_list(startPtr);
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(startPtr, key);
getch();
return 0;
}
int create_list(int *arr)
{
int i;
for(i=0; i<10; i++)
{
printf("Enter an integer [%d]", i+1);
scanf("%d", &arr[i]);
}
return 0;
}
int add_node(int *arr)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr;
for(i=0; i<10; i++)
{
newPtr = malloc(sizeof(listNodePtr)
newPtr->data = arr[i];
newPtr->nextPtr = NULL;
if(startPtr == NULL)
startPtr = newPtr;
else
currPtr = startPtr;
while(currPtr-> nextPtr != NULL)
currPtr = currPtr->nextPtr;
currPtr->nextPtr = newPtr;
}
return 0;
}
int del_node(listNodePtr startPtr, int key)
{
int i;
listNodePtr currPtr, tempPtr;
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
currPtr = startPtr;
while(currPtr != NULL)
{
tempPtr = currPtr;
if(tempPtr->data == key)
{
currPtr->nextPtr = tempPtr->nextPtr; /* change per comment and still not working */
free(tempPtr);
tempPtr->data = 0; /* dummy line */
currPtr = currPtr->nextPtr;
}
else
currPtr = currPtr->nextPtr;
}
display_list(startPtr);
return 0;
}
int display_list(listNodePtr startPtr)
{
listNodePtr currPtr;
currPtr = startPtr;
while(currPtr != NULL)
{
printf("%d->", currPtr->data);
currPtr = currPtr-> nextPtr;
}
printf("NULL\n");
return 0;
}
******************
#include <stdio.h>
#include <stdlib.h>
#include <alloc.h>
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int create_list(int *);
int add_node(int *);
int del_node(listNodePtr, int);
int display_list(listNodePtr);
int main()
{
int arr[10], key, i;
listNodePtr startPtr;
create_list(arr);
add_node(arr);
display_list(startPtr);
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(startPtr, key);
getch();
return 0;
}
int create_list(int *arr)
{
int i;
for(i=0; i<10; i++)
{
printf("Enter an integer [%d]", i+1);
scanf("%d", &arr[i]);
}
return 0;
}
int add_node(int *arr)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr;
for(i=0; i<10; i++)
{
newPtr = malloc(sizeof(listNodePtr)
newPtr->data = arr[i];
newPtr->nextPtr = NULL;
if(startPtr == NULL)
startPtr = newPtr;
else
currPtr = startPtr;
while(currPtr-> nextPtr != NULL)
currPtr = currPtr->nextPtr;
currPtr->nextPtr = newPtr;
}
return 0;
}
int del_node(listNodePtr startPtr, int key)
{
int i;
listNodePtr currPtr, tempPtr;
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
currPtr = startPtr;
while(currPtr != NULL)
{
tempPtr = currPtr;
if(tempPtr->data == key)
{
currPtr->nextPtr = tempPtr->nextPtr; /* change per comment and still not working */
free(tempPtr);
tempPtr->data = 0; /* dummy line */
currPtr = currPtr->nextPtr;
}
else
currPtr = currPtr->nextPtr;
}
display_list(startPtr);
return 0;
}
int display_list(listNodePtr startPtr)
{
listNodePtr currPtr;
currPtr = startPtr;
while(currPtr != NULL)
{
printf("%d->", currPtr->data);
currPtr = currPtr-> nextPtr;
}
printf("NULL\n");
return 0;
}
Sunny,
I read the "Member Comments" that you have posted.
Thanks for that. I should not have posted the correct code in such a hurry.
Ok, I will take your advice in right spirit and help the asker:-)
-ssnkumar
I read the "Member Comments" that you have posted.
Thanks for that. I should not have posted the correct code in such a hurry.
Ok, I will take your advice in right spirit and help the asker:-)
-ssnkumar
Hi Emms,
Hope you have not read my previous comments:-)
I will give some tips for you to improve your code:
1. Make startPtr as a global variable.
2. In the add_node() function, put all the statements under else part in the same block. These should not get executed when the if condition becomes TRUE.
3. In the del_node() function, the search has to go one step in advance. What I mean is search for startPtr->nextPtr->data and when the data matches, delete this node and adjust the pointer. Also, since startPtr is a function parameter to this function, the global strPtr will not get affected and hence no need to store the value in a temp variable.
4. Move the Display_node() inside the delete_node() to main() function.
Do change your code based on my tips and give your feedback. If your code is still giving problems, then it will be better to post the latest code.
-ssnkumar
Hope you have not read my previous comments:-)
I will give some tips for you to improve your code:
1. Make startPtr as a global variable.
2. In the add_node() function, put all the statements under else part in the same block. These should not get executed when the if condition becomes TRUE.
3. In the del_node() function, the search has to go one step in advance. What I mean is search for startPtr->nextPtr->data and when the data matches, delete this node and adjust the pointer. Also, since startPtr is a function parameter to this function, the global strPtr will not get affected and hence no need to store the value in a temp variable.
4. Move the Display_node() inside the delete_node() to main() function.
Do change your code based on my tips and give your feedback. If your code is still giving problems, then it will be better to post the latest code.
-ssnkumar
ASKER
*********************
Sunny,
I read the "Member Comments" that you have posted.
Thanks for that. I should not have posted the correct code in such a hurry.
Ok, I will take your advice in right spirit and help the asker:-)
-ssnkumar
**********************
ssnkumar, what correct code have you posted in such a hurry?? I didn't see it. Please do not take credit for work you didn't do. Your previous comment:-
"Please do reorganise your code and then we will look for a solution. Organize your code into 3 functions: create_list, add_node, delete_node and display_list."
Sunny,
I read the "Member Comments" that you have posted.
Thanks for that. I should not have posted the correct code in such a hurry.
Ok, I will take your advice in right spirit and help the asker:-)
-ssnkumar
**********************
ssnkumar, what correct code have you posted in such a hurry?? I didn't see it. Please do not take credit for work you didn't do. Your previous comment:-
"Please do reorganise your code and then we will look for a solution. Organize your code into 3 functions: create_list, add_node, delete_node and display_list."
ASKER
1, do not intend to change structure of codings, aim is to fix codings
2, not applicable as there is only 1 statement under else
3, will be back on this
4, harmless though good programming practice
2, not applicable as there is only 1 statement under else
3, will be back on this
4, harmless though good programming practice
ASKER
sunny, snippet below is to rule out every key in the "first" node, i.e. startPtr always points to and start at first non-key node in the list.
********
your current source of problems seems to be this code snippet
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
I am not sure if you want this condition in while ... perhaps you are looking for the key and thus you should be using != in place of == or if in place of while (?? logic please)
********
your current source of problems seems to be this code snippet
currPtr = startPtr;
tempPtr = startPtr;
while(startPtr->data == key)
{
tempPtr = startPtr;
startPtr = currPtr->nextPtr;
currPtr = currPtr->nextPtr;
free(tempPtr);
}
I am not sure if you want this condition in while ... perhaps you are looking for the key and thus you should be using != in place of == or if in place of while (?? logic please)
ASKER
thanks for clearing things up, sunnycoder n ssnkumar apologies for the whip
Emms
Emms
>currPtr = startPtr;
>tempPtr = startPtr;
Only tempPtr is enough (you will need it only when you want to free memory for a node). currPtr is not needed.
>while(startPtr->data == key)
The while loop has to go on till you don't find the key.....Also, check one step ahead (otherwise you will have to come back one step to get the address of the current node and you will need one more pointer...currPtr!)
So, the correct code should be:
while(startPtr->nextPtr->d
> free(tempPtr);
You need to free the address only after finding the correct node.
So, move this free() to outside the loop.
One more clarification.....do you want to delete a node or you just want to make the contents of the node as NULL?
-ssnkumar
>tempPtr = startPtr;
Only tempPtr is enough (you will need it only when you want to free memory for a node). currPtr is not needed.
>while(startPtr->data == key)
The while loop has to go on till you don't find the key.....Also, check one step ahead (otherwise you will have to come back one step to get the address of the current node and you will need one more pointer...currPtr!)
So, the correct code should be:
while(startPtr->nextPtr->d
> free(tempPtr);
You need to free the address only after finding the correct node.
So, move this free() to outside the loop.
One more clarification.....do you want to delete a node or you just want to make the contents of the node as NULL?
-ssnkumar
> every key in the "first" node
I believe there is only one key at each node
int main()
{
int arr[10], key, i;
listNodePtr startPtr;
You declare startPtr in main ...
and as global variable
>listNodePtr startPtr = NULL;
The startPtr delcared in main is a different variable from startPtr declared globally ... When your control is inside main, the local declaration takes precedence and is used for all references to startPtr
Thus when you say,
display_list(startPtr);
del_node(startPtr, key);
you actually pass the uninitialized local variable ...
I believe there is only one key at each node
int main()
{
int arr[10], key, i;
listNodePtr startPtr;
You declare startPtr in main ...
and as global variable
>listNodePtr startPtr = NULL;
The startPtr delcared in main is a different variable from startPtr declared globally ... When your control is inside main, the local declaration takes precedence and is used for all references to startPtr
Thus when you say,
display_list(startPtr);
del_node(startPtr, key);
you actually pass the uninitialized local variable ...
ASKER
>> every key in the "first" node
>I believe there is only one key at each node
what i mean to say is if node1 contains the key, node1 will be freed and startPtr takes the position of node2. thus node2 becomes the "first" node of the list since node1 is freed... and so on.
when is startPtr said to be declared globally? please explain.
>I believe there is only one key at each node
what i mean to say is if node1 contains the key, node1 will be freed and startPtr takes the position of node2. thus node2 becomes the "first" node of the list since node1 is freed... and so on.
when is startPtr said to be declared globally? please explain.
ASKER
got through deleting the key nodes.
ASKER
stuck on passing startPtr only.
If you want to retain the value stored in startPtr after modifications in function call, then pass the address of the pointer.
-ssnkumar
-ssnkumar
Doh .... it seems that I read half of the code from your post and the remaining half from ssnkumar's post (I can still see it :o) )
This was the last code you posted
==========================
#include <stdio.h>
#include <stdlib.h>
#include <alloc.h>
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int create_list(int *);
int add_node(int *);
int del_node(listNodePtr, int);
int display_list(listNodePtr);
int main()
{
int arr[10], key, i;
listNodePtr startPtr; ---- uninitialzed variable
Also when you call create_list or add_node or any other functions, they do not know anything at all about startPtr ... thus till the end of the program, it will have some junk value and you will get undefined results ...
There are three ways to inform other functions of existence of this variable
1. make it global .... all will see and update the same copy .... not recommended
2. pass pointer to this pointer to all functions ... beginners often get lost while using this way ... so be careful
3. pass this pointer to the functions and return the updated value from the functions
something like
startPtr = add_node ( arr, startPtr);
create_list(arr);
add_node(arr);
display_list(startPtr);
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(startPtr, key);
getch();
return 0;
}
int add_node(int *arr)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr;
again this startPtr is local to this function .. it is destroyed as soon as control exits this function ... Functions outside add_node do not know of it !!!
This was the last code you posted
==========================
#include <stdio.h>
#include <stdlib.h>
#include <alloc.h>
struct listNode {
int data;
struct listNode *nextPtr;
};
typedef struct listNode listNode;
typedef listNode *listNodePtr;
int create_list(int *);
int add_node(int *);
int del_node(listNodePtr, int);
int display_list(listNodePtr);
int main()
{
int arr[10], key, i;
listNodePtr startPtr; ---- uninitialzed variable
Also when you call create_list or add_node or any other functions, they do not know anything at all about startPtr ... thus till the end of the program, it will have some junk value and you will get undefined results ...
There are three ways to inform other functions of existence of this variable
1. make it global .... all will see and update the same copy .... not recommended
2. pass pointer to this pointer to all functions ... beginners often get lost while using this way ... so be careful
3. pass this pointer to the functions and return the updated value from the functions
something like
startPtr = add_node ( arr, startPtr);
create_list(arr);
add_node(arr);
display_list(startPtr);
printf("Enter a number to delete: ");
scanf("%d", &key);
del_node(startPtr, key);
getch();
return 0;
}
int add_node(int *arr)
{
int i;
listNodePtr startPtr=NULL, newPtr, currPtr;
again this startPtr is local to this function .. it is destroyed as soon as control exits this function ... Functions outside add_node do not know of it !!!
Please do reorganise your code and then we will look for a solution.
Organize your code into 3 functions: create_list, add_node, delete_node and display_list.
Once you achieve this, then I think you would have found the solution yourself.
If you want help to reorganise the code as I have suggested, please do revert back and we will discuss furthur.
-ssnkumar