There are a multitude of little problems with the code in the question, but when those are cleaned up, the core algorithms seem to work correctly. The indentation is a bit curious in places; I'll ignore that from here on.
The critique can start rather quickly, unfortunately:
struct listnode
{
char data[100];
listnode *nextPtr;
};
typedef struct listnode ListNode;
typedef ListNode *ListNodePtr;
This is a C question, but if the code ever compiled, it was because it was compiled by a C++ compiler. The listnode *nextPtr;
is not valid in C (but would be in C++). At that line, the name struct listnode
is known, but the type listnode
without the struct
prefix is not defined in any of the standard headers (under normal circumstances, at any rate). You need struct listnode *nextPtr;
to be valid C. Or you need to move the typedef struct listnode ListNode;
before the structure definition, and then use ListNode *nextPtr;
in the structure. You should probably not define or use ListNodePtr
— see Is it a good idea to typedef pointers? for a discussion of why. You might clean up the code like this. Note that structure tags (like ListNode
) are in the tags namespace (along with union
tags and enum
tags), but the typedef name ListNode
is in the distinct 'ordinary identifiers' namespace — so there is no conflict between the uses of ListNode
in this code:
typedef struct ListNode ListNode;
struct ListNode
{
char data[100];
ListNode *nextPtr;
};
In what follows, 'cleaning up the types' refers to using ListNode
instead of ListNodePtr
or any other variant spellings.
After cleaning up the types, the code in insert()
is OK; the error message should be printed to stderr
, and spell-checked, and should end with a newline.
The code in del()
is probably not in the function delete()
because delete
is a keyword in a C++ compiler and hence can't be used as a function name. Don't use C++ compilers to compile C — you get misleading results.
The declared return type of the function is const char *
, but the returned values are not pointers. It would be best to change the function to return either an int
or even a bool
to indicate whether it successfully deleted the specified name.
There was also a spacing blunder at:
previousPtr - > nextPtr = currentPtr->nextPtr;
There shouldn't be spaces around either the dot .
or arrow ->
operators, and with a space between the -
and the >
you don't even formally have an arrow operator.
However, the core algorithm in del()
seems to work OK. There probably is room for improvement.
Your code referenced undefined (and indeed undeclared) functions like Empty()
, printList()
, instructions()
— when you are creating an MCVE (Minimal, Complete, Verifiable Example), you should not have such omissions. The code below provides simple implementations for each of these.
The code in main()
showed a variety of problems. One is code repetition by not writing useful functions to do particular pieces of work. I don't know what your plan for the instructions()
function was, but I retained it and reduced it to a single printf()
that prints a single prompt (without a newline at the end). It is itself used from a function get_option()
that deals with a number of fiddly details.
You should NEVER use the gets()
function; read about why it is far too dangerous to use gets()
— ever!. Plus, mixing scanf()
with functions like fgets()
is a bit fraught because scanf()
leaves the newline in the input buffer. I created the functions get_option()
and get_string()
to handle the issues, and to deal with invalid (non-numeric) input, and unexpected EOF, etc. They also flush standard output but not standard input. See Using fflush(stdin)
for a moderately nuanced discussion — suffice to say fflush(stdin)
is not portable, though it does have defined behaviour on Windows, but it does very little anywhere else other than invoke undefined behaviour. Generally, avoid it. If you must use it, be aware that you limit the portability of your code (and it is generally unnecessary to do that).
With the 'loop with input and test at the top', the code in the body of main()
is slightly simpler. I resequenced the tests in the case 2:
code to avoid nesting ifs; it is generally best to do something after testing, rather than starting a new test.
Note that the error condition reports the value that was entered (read). This is valuable to users — if you see something unexpected as the reported value, you may be able to deduce what went wrong better.
Putting it all together gives you code like this:
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct ListNode ListNode;
struct ListNode
{
char data[100];
ListNode *nextPtr;
};
extern void insert(ListNode **strPtr, char value[]);
extern bool delete(ListNode **strPtr, char value[]);
void insert(ListNode **strPtr, char value[])
{
ListNode *previousPtr, *currentPtr, *newPtr;
newPtr = (ListNode *)malloc(sizeof(ListNode));
strcpy(newPtr->data, value);
newPtr->nextPtr = NULL;
previousPtr = NULL;
currentPtr = *strPtr;
if (newPtr != NULL)
{
while (currentPtr != NULL &&
strcmp(currentPtr->data, value) < 0)
{
previousPtr = currentPtr;
currentPtr = currentPtr->nextPtr;
}
if (previousPtr == NULL)
{
newPtr->nextPtr = *strPtr;
*strPtr = newPtr;
}
else
{
previousPtr->nextPtr = newPtr;
newPtr->nextPtr = currentPtr;
}
}
else
fprintf(stderr, "%s was not inserted. Insufficient memory!\n", value);
}
bool delete(ListNode **strPtr, char value[])
{
ListNode *previousPtr, *currentPtr, *tempPtr;
if (strcmp(value, (*strPtr)->data) == 0)
{
tempPtr = *strPtr;
*strPtr = (*strPtr)->nextPtr;
free(tempPtr);
return true;
}
else
{
previousPtr = *strPtr;
currentPtr = (*strPtr)->nextPtr;
while (currentPtr != NULL && strcmp(value, currentPtr->data) != 0)
{
previousPtr = currentPtr;
currentPtr = currentPtr->nextPtr;
}
if (currentPtr != NULL)
{
tempPtr = currentPtr;
previousPtr->nextPtr = currentPtr->nextPtr;
free(tempPtr);
return true;
}
}
return false;
}
static bool Empty(ListNode *ptr)
{
return(ptr == NULL);
}
static void get_string(size_t size, char buffer[size])
{
if (fgets(buffer, size, stdin) == 0)
{
fprintf(stderr, "Unexpected EOF on standard input\n");
exit(EXIT_FAILURE);
}
buffer[strcspn(buffer, "\n")] = '\0';
}
static void instructions(void)
{
printf("1 to add, 2 to delete, 3 to exit: ");
}
static int get_option(void)
{
int optiune;
instructions();
fflush(stdout);
if (scanf("%d", &optiune) != 1)
{
fprintf(stderr, "Failed to read option number\n");
exit(EXIT_FAILURE);
}
int c;
while ((c = getchar()) != EOF && c != '\n')
;
return optiune;
}
static void printList(ListNode *ptr)
{
for (int i = 0; ptr != NULL; i++)
{
printf("%d: %s\n", i, ptr->data);
ptr = ptr->nextPtr;
}
}
int main(void)
{
ListNode *startPtr;
startPtr = NULL;
int optiune;
char nume[100];
while ((optiune = get_option()) != 3)
{
switch (optiune)
{
case 1:
printf("Enter name: ");
fflush(stdout);
get_string(sizeof(nume), nume);
insert(&startPtr, nume);
printList(startPtr);
break;
case 2:
printf("Search by name to delete from list: ");
fflush(stdout);
get_string(sizeof(nume), nume);
if (Empty(startPtr))
printf("List is empty!\n");
else if (delete(&startPtr, nume))
{
printf("%s was deleted!\n", nume);
printList(startPtr);
}
else
printf("%s was not found!\n", nume);
break;
default:
fprintf(stderr, "No such option (%d)!\n\n", optiune);
break;
}
}
printf("Execution stopped.\n");
return EXIT_SUCCESS;
}
(I make functions static
unless there's a header to declare them in, and another source file that will use them. This can even help with optimization since the compiler can tell how often a static
function is called and my inline the code.)
Here's a sample run of the code above:
1 to add, 2 to delete, 3 to exit: 1
Enter name: Patricia
0: Patricia
1 to add, 2 to delete, 3 to exit: 1
Enter name: Persephone
0: Patricia
1: Persephone
1 to add, 2 to delete, 3 to exit: 1
Enter name: Piglet
0: Patricia
1: Persephone
2: Piglet
1 to add, 2 to delete, 3 to exit: 1
Enter name: Pooh
0: Patricia
1: Persephone
2: Piglet
3: Pooh
1 to add, 2 to delete, 3 to exit: 1
Enter name: Puss In Boots
0: Patricia
1: Persephone
2: Piglet
3: Pooh
4: Puss In Boots
1 to add, 2 to delete, 3 to exit: 1
Enter name: Pygmalion
0: Patricia
1: Persephone
2: Piglet
3: Pooh
4: Puss In Boots
5: Pygmalion
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Pooh
Pooh was deleted!
0: Patricia
1: Persephone
2: Piglet
3: Puss In Boots
4: Pygmalion
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Pygmalion
Pygmalion was deleted!
0: Patricia
1: Persephone
2: Piglet
3: Puss In Boots
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Patricia
Patricia was deleted!
0: Persephone
1: Piglet
2: Puss In Boots
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Piglet
Piglet was deleted!
0: Persephone
1: Puss In Boots
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Puss In Boots
Puss In Boots was deleted!
0: Persephone
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Quantum Gold
Quantum Gold was not found!
1 to add, 2 to delete, 3 to exit: 2
Search by name to delete from list: Persephone
Persephone was deleted!
1 to add, 2 to delete, 3 to exit: 3
Execution stopped.