Deleting first node in linked list

Question

I am having problems with my homework, which deals with editing linked list. My code seems to work fine, except it does not delete data items with student ID=1 (or the first node in linked list). When I try to delete the first node on the linked list (or, delete(1) within main()), it returns "Student not found" message. Could someone please tell me what went wrong? Thanks.

/****************************************************************************  
    Date        : 12 August 2015  
    Assignment  : Use linked list to build a database of student records, which supports find(), insert() and delete()  
    ****************************************************************************/

#include <stdlib.h>
#include <stdio.h>

/* Define structure studentNode that contains ID, name, GPA and pointer to next studentNode*/
typedef struct student {
    int ID;
    char *name;
    float GPA;
    struct student *nextStudentNode;
} studentNode;

/* Initialize header */
studentNode *header;

/* Function prototypes*/
void insert(int ID, char *name, float GPA);
void printList();
void delete(int ID);

/* Main function which runs through while loop until sentinel value of q is typed by user. */
int main() {
    insert(3, "John", 3.5);
    insert(12, "Smith", 2.8);
    insert(1, "Mary", 1.8);

    printList();

    delete(1);    /***** there seems to be a problem here*********/
    delete(12);
    delete(3);
    printList();

    return 0;
}

/* Insert new student data */
void insert(int ID, char *name, float GPA) {
    studentNode *newStudent = (studentNode*)malloc(sizeof(struct student));

    newStudent->ID          = ID;
    newStudent->name        = name;
    newStudent->GPA         = GPA;

    newStudent->nextStudentNode = header; 
    header = newStudent;
}

/* Run through linked list one by one to print all data items */
void printList() {
    studentNode *tempNode;
    for (tempNode = header ; tempNode != NULL ; tempNode = tempNode->nextStudentNode) {
        printf( "ID:%d name:%s GPA:%f\n" , tempNode->ID, tempNode->name, tempNode->GPA);
    }
}

/* Delete student data */
void delete(int inputID) {
    studentNode *tempNode;
    studentNode **headerPtr = &header;

    for (tempNode = *headerPtr ;
         tempNode->nextStudentNode->ID != inputID && tempNode->nextStudentNode->nextStudentNode != NULL;
         tempNode = tempNode->nextStudentNode);

    studentNode *nodeToDelete = tempNode->nextStudentNode;

    if (nodeToDelete->ID != inputID) {
        printf("Student not found.\n");
    } else {
        printf("ID:%d name:%s GPA:%f is deleted.\n" , nodeToDelete->ID, nodeToDelete->name,
               nodeToDelete->GPA);

        tempNode->nextStudentNode = nodeToDelete->nextStudentNode;
        free(nodeToDelete);
    }
}

Time to learn how to use a debugger, and how to step though the code, line by line. That way you can easily see what happens and what the values of all variables are and what they change to (if they change). — Some programmer dude, Aug 13 '15 at 18:11
There are a few things in your code that looks suspicious, for example in the `delete` function, what it the use of `headerPtr`? It's used only once, and in that place you don't need a pointer to a pointer to your node structure. You also don't check for empty lists, or if `tempNode` is `NULL` after the loop. You also don't explicitly initialized the `header` pointer, which works only because it's a global variable, if you modify your program so that you pass the head pointer to the functions and make the `header` variable a local variable that won't work anymore. — Some programmer dude, Aug 13 '15 at 18:14
And the standard note: [In C you should not cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Or any function returning `void*` for that matter. — Some programmer dude, Aug 13 '15 at 18:17
@JoachimPileborg Thanks. I followed your advice and remove malloc cast & initialized header pointer as NULL & got rid of unnecessary headerPtr. But because I'm a newbie, I am not quite familiar with debugging process. Can you briefly explain how I can do that? — Bossam, Aug 14 '15 at 04:22

score 1 · Answer 1 · answered Aug 13 '15 at 18:11

1

The problem is in this for:

for( tempNode = *headerPtr ; tempNode->nextStudentNode->ID != inputID && tempNode->nextStudentNode->nextStudentNode != NULL ; tempNode = tempNode->nextStudentNode );

You are starting checking from second student, not for the first one. So you are going to the end of the list without finding a proper ID.

UPDATE: I mean the list order.

answered Aug 13 '15 at 18:11

jan_kowalski_314

84
7

1

As @Joachim Pileborg said in your code we can see a lot of mistakes, but this one is primary. That's the reason of "Student not found" message. – jan_kowalski_314 Aug 13 '15 at 18:22

Vlad from Moscow · Accepted Answer · 2015-08-13T18:35:16.133

This condition in the first loop of the function

tempNode->nextStudentNode->ID != inputID && tempNode->nextStudentNode->nextStudentNode != NULL ;

is wrong because it skips ID in the first node.

The function can be written simpler

/* Delete student data */
void delete( int inputID )
{
    studentNode *prev = NULL;
    studentNode *tempNode = head;

    while ( tempNode && tempNode->ID != inputID )
    {
        prev = tempNode;
        tempNode = tempNpde->nextStudentNode;
    }

    if ( tempNode )
    {
        if ( !prev ) head = tempNode->nextStudentNode;
        else prev->nextStudentNode = tempNode->nextStudentNode;

        free( tempNode );
    }
}

Deleting first node in linked list

2 Answers2