Reputation: 23
Double linked list. Am i doing it ok?
I'm a beginner in C programming. I had a task creating a list of students using a double linked list. The application should have three points: display the list, add a new student and delete a student by his ID number. I did it and it runs very well. I would like to ask a few questions:
- Is something used inappropriate?
- If there is room to shorten the code I would be happy to recieve any suggestions.
Here is my code:
struct student
{
char first_name[20];
char last_name[20];
char ID_NO[14];
struct student* previous;
struct student* next;
};
void Menu();
void Show_List(struct student*);
struct student* Add_Student(char [], char [], char [], struct student*);
struct student* Erase_Student(char [], struct student*);
void main(void)
{
char student_first_name[20];
char student_last_name[20];
char personal_code[14];
struct student* first = NULL;
struct student* node0 = NULL;
int x = 0;
int opt;
Menu();
for(; ;)
{
printf("\nEnter the operation you want to do: \n");
scanf("%d", &opt);
switch(opt)
{
case 1:
Show_List(first);
break;
case 2:
printf("\nEnter the student's first name: ");
scanf("%s", &student_first_name);
printf("\nEnter the student's last name: ");
scanf("%s", &student_last_name);
printf("\nEnter the student's personal code: ");
scanf("%s", &personal_code);
node0 = Add_Student(student_first_name, student_last_name, personal_code, first);
first = node0;
break;
case 3:
printf("\nEnter the code of the student you want to erase: ");
scanf("%s", &personal_code);
node0 = Erase_Student(personal_code, first);
first = node0;
break;
default:
printf("\nYou entered an invalid option!!! Please try again.\n");
Menu();
break;
}
}
scanf("%d", &x);
}
void Menu()
{
printf("\nSelect from the Menu the operation you want to execute:\n");
printf("\n1) Show students list;");
printf("\n2) Add a student in the list;");
printf("\n3) Erase a student from the list.");
}
void Show_List(struct student* firstNode)
{
struct student* firstNodeCopy = firstNode;
int number = 0;
if(firstNode == NULL)
printf("\nThe list is empty.\n");
while(firstNodeCopy)
{
printf("\n%d. %s ", ++number, firstNodeCopy->first_name);
printf("%s %s\n", firstNodeCopy->last_name, firstNodeCopy->ID_NO);
firstNodeCopy = firstNodeCopy->next;
}
}
struct student* Add_Student(char name_1[], char name_2[], char ID[], struct student* firstNode)
{
struct student* start = firstNode;
struct student* last = NULL;
struct student* addNode = (struct student*) malloc(sizeof(struct student));
if(firstNode == NULL)
{
firstNode = (struct student*) malloc(sizeof(struct student));
strcpy(firstNode->first_name, name_1);
strcpy(firstNode->last_name, name_2);
strcpy(firstNode->ID_NO, ID);
firstNode->next = NULL;
firstNode->previous = NULL;
return firstNode;
}
else
{
strcpy(addNode->first_name, name_1);
strcpy(addNode->last_name, name_2);
strcpy(addNode->ID_NO, ID);
while(start)
{
if(strcmp(addNode->first_name, start->first_name) > 0)
{
if(start->next == NULL)
{
start->next = addNode;
addNode->previous = start;
addNode->next = NULL;
return firstNode;
}
else
{
last = start;
start = start->next;
}
}
if(strcmp(addNode->first_name, start->first_name) < 0)
{
if(last == NULL)
{
addNode->next = start;
start->previous = addNode;
return addNode;
}
else
{
addNode->next = start;
addNode->previous = last;
last->next = addNode;
start->previous = addNode;
return firstNode;
}
}
if(strcmp(addNode->first_name, start->first_name) == 0)
{
if(strcmp(addNode->last_name, start->last_name) < 0)
{
if(last == NULL)
{
addNode->next = start;
start->previous = addNode;
return addNode;
}
else
{
addNode->next = start;
addNode->previous = last;
last->next = addNode;
start->previous = addNode;
return firstNode;
}
}
if(strcmp(addNode->last_name, start->last_name) > 0)
{
if(start->next == NULL)
{
start->next = addNode;
addNode->previous = start;
addNode->next = NULL;
return firstNode;
}
else
{
last = start;
start = start->next;
}
}
if(strcmp(addNode->last_name, start->last_name) == 0)
{
if(last == NULL)
{
addNode->next = start;
start->previous = addNode;
return addNode;
}
else
{
addNode->next = start;
addNode->previous = last;
last->next = addNode;
start->previous = addNode;
return firstNode;
}
}
}
}
}
}
struct student* Erase_Student(char ID[], struct student* firstNode)
{
struct student* start = firstNode;
struct student* last = NULL;
if(start == NULL)
{
printf("\nThe list is empty.\n");
return NULL;
}
else
{
while(start)
{
if(strcmp(ID, start->ID_NO) == 0)
{
if(last == NULL)
{
start = start->next;
return start;
}
else
{
last->next = start->next;
return firstNode;
}
}
else
{
last = start;
start = start->next;
}
}
printf("\nYou entered a WRONG personal ID number to erase!!! Please try again.\n");
return firstNode;
}
}
Upvotes: 2
Views: 803
Answers (8)
Reputation: 88
You could change the prototype of void Menu() to void Menu(void). This way the compiler can check whether the function is called correctly or not. It is always good if errors can already be found at compile time.
Upvotes: 0
Reputation: 108938
Some nitpicks I found
#include: in this program you need<stdio.h>,<stdlib.h>, and<string.h>- magic numbers: use
enumor#definerather than sprinking your source with "magic numbers" - pay attention to buffer overflows when using
scanfand check return value - style: prefer
'\n'at end of output:printf("...\n")vsprintf("\n...") - try not to repeat the
strcmps
Compile with warning level pushed to the MAX, and try to get rid of all warnings.
Upvotes: 0
Reputation: 490178
You choices of a number of variable names make the code unnecessarily confusing. For example, in Show_List, you have something named firstNodeCopy that isn't a copy of any node, and doesn't always refer to the first node.
In Add_Student, you have so many different comparisons I'm not even sure what you're really even trying to accomplish, but I'm pretty sure what you have is open to improvement. Again, you have some variables (e.g., start) whose names don't seem to be a very solid indication of what they're really supposed to do/be.
I'd advise changing the structure a bit: create a function compare_students (or something like that) that just handles comparing one student to another, and (for example) telling you whether two students A and B are in order or out of order. I'd probably also add a "create student" to take a set of inputs and create a student structure from them. From there, the insertion algorithm looks something like:
if list is empty, put new node in list
if new node precedes first item in list, insert at head of list
Walk through list until end of list or current node precedes new node
if at end of list, add new node to end
else insert new node before current node
Upvotes: 0
Reputation:
- Your use of scanf is unsafe (input can overflow the destination char array). Try
scanf("%19s", &student_first_name); - Using strcpy is unsafe, too. Look at strncpy.
Upvotes: 0
Reputation: 91270
- You have a memory leak when you add the first node, you allocate both addNode and firstNode
- You have a memory leak when you erase a student, you need to
free()the removed node. - You should use a function to compare names instead of the duplicated code. Something like
int compareStudents(char * LeftFirstName, char * LeftLastName, char * RightFirstName, char * RightLastName);
Apart from that, the logic is good.
Upvotes: 2
Reputation: 111
Take a quick look at the Add_Student(...) function. Looks like a small memory leak. I can be more specific if you want, but thought you may want the practice.
Upvotes: 0
Reputation: 76949
That's actually very good! After your introduction, I was hoping for a mess, I admit.
You did a very good job there, man. Whatever you can improve, you'll learn in due time :). Keep working like that!
I was about to suggest you look up what "abstract data types" are, and how to expose interfaces obscuring implementation, but as I said -- you'll learn that soon enough!
Cheers.
Upvotes: 0
Reputation: 505
You did everything perfectly well! Nice job, for the beginning! Keep learning.
Upvotes: 0
Related Questions
- Double linked list2
- i want to make sure that my linked list work
- Correctly Implementing a Linked List in C
- Implementing linked list inside linked list in c
- Linked List In Data Structures
- Linked list and double pointers
- Double linked list implementation in c
- Simple LInked list
- Linked list within a structure
- Single linked list