Sorting array from typedef struct in C

You can use the already implemented sorting function qsort function available at stdlib.h:

int SortFunc(void* a, void* b) {    phone *p1 = (phone*)a;    phone *p2 = (phone*)b;    return strcmp(p1->Surname, p2->Surname);}void Sort (phone phonebook[]) {    qsort(phonebook, counter, sizeof(phone), &SortFunc);} 

The function is usually Quicksort, but that's up to the C library implementation to decide.

Update:

The blank listing is because the sorting is reversed and always sorting all the 19 items of the phonebook, comparing the empty ones against the real ones. If you have less than 19 entries on the phonebook, the actual data is going to be present at the end of the phonebook array.

Your original Sort function was always working almost OK. Just change the end condition on the two for.

void Sort (phone phonebook[]) {    phone temp;    int i;  int j;    for (i=0; i<counter; i++) {        for (j=i+1; j<counter; j++) {            if (strcmp(phonebook[i].Surname, phonebook[j].Surname) > 0) {                temp=phonebook[i];                phonebook[i]=phonebook[j];                phonebook[j]=temp;            }        }    }}

I've also updated my Sort above.

First things first, you have a buffer overflow issue here:

char userChoice;:scanf("%s", &userChoice);

That scanf will write two bytes when you enter one character (the character plus a null terminator). This corrupted the first name of the first phonebook entry in my environment but, since it's undefined behaviour, it could do anything!

You can get around this by using:

char userChoice[] = "something that's not Q";: scanf("%s", userChoice);:if (*userChoice == 'A')  // for all of these.

That won't stop a buffer overflow if you enter enough text but it will if you limit yourself to single character commands. If you want a truly robust user input function, see here.

Now to your specific problem. It looks like you have a bit of a bubble sort going on there, but your logic is slightly off. Assuming you don't want to use qsort (which would be the better way for real code), you just need to fix up a couple of things.

Your outer loop is okay, as is your inner loop, but the inner loop body should be comparing elements j and j+1, not j and i. That's because it works by swapping adjacent elements if they're out of order.

In addition, a forward focused bubble sort will place the highest element at the end of the list on the first pass, so you can't start j at i+1 on the second pass, simply because the first element may not be correct yet.

The following psuedo-code is your classic bubble sort:

didSwap = truewhile didSwap:    didSwap = false    for i = 0 to lastidx - 1:        if array[i] > array[i+1]:            temp = array[i]            array[i] = array[i+1]            array[i+1] = temp            didSwap = true

Read that, understand how it works, then implement it on your own. If you have trouble with that, I've included a working version below:

void Sort (phone phonebook[]) {    phone temp;    int i;  int didSwap;    didSwap = 1;    while (didSwap) {        didSwap = 0;        for (i = 0; i < counter - 1; i++) {            if (strcmp(phonebook[i].Surname, phonebook[i+1].Surname) > 0) {                temp=phonebook[i];                phonebook[i]=phonebook[i+1];                phonebook[i+1]=temp;                didSwap = 1;            }        }    }}

for (i=0; i<19; i++){     for (j=i+1; j<19; j++)    {        if (strcmp(phonebook[i].Surname, phonebook[j].Surname) > 0)        {            temp=phonebook[i];            phonebook[i]=phonebook[j];            phonebook[j]=temp;        }    }}

Three problems with your code. First is the logic of your algorithm. Bubble sort works by fixing the order of two adjacent element. In your code, after the first iteration of your inner for loop, it's not going to compare two adjacent elements.

The second problem, again in sorting algorithm, your counters i and j are both going to 19, even when there is less entries than that. This might mess up the sorting as they will be reading invalid(uninitialized) entry. you should check the upper bound for the counter.

The next one is in the deletion

    if (strcmp(deleteName, phonebook[x].Name) == 0) //compare deleteName to phonebook.Name     {        for (x = 0; x < counter; x++)        {            if (strcmp(deleteSurname, phonebook[x].Surname) == 0) //If deleteSurname matches phonebook.Surname            {                strcpy(phonebook[x].Name, nullStr); //Put null into Name                strcpy(phonebook[x].Surname, nullStr); //Null into Surname                strcpy(phonebook[x].PhoneNumber, nullStr); //Null into PhoneNumber                printf("Contact removed from phonebook.\n");                counter--;                break;            }        }    }

The code above will not check properly whether the first and last name since you're checking them separately. You only need one for loop with if( strcmp(deleteSurname, phonebook[x].Surname) == 0 && strcmp(deleteName, phonebook[x].Name) == 0 )