Sorting data with array of pointers to a structure using strcmp

Question

In my program, I create an array of pointers to a struct (typedef'd as Person) and sort information in ascending order by zip code.

Here is the function that is supposed to sort my data:

void sortdata(Person * arr[], int noElements) {
    /* temporary pointer to Person data type to aid with swapping */
    Person * tempptr = (Person * ) malloc(sizeof(Person));

    int i, j, compare;

    for (i = 0; i <= (noElements - 1); i++); {
        for (j = (i + 1); j <= noElements; j++) {
            compare = strcmp(arr[i] - > zip, arr[j] - > zip);
            if (compare > 0) {
                printf("attempted sort %d times.\n", j);
                /* stores value in index i for array inside of temporary pointer  */
                strcpy(tempptr - > name, arr[i] - > name);
                strcpy(tempptr - > address, arr[i]);
                strcpy(tempptr - > citystate, arr[i] - > address);
                strcpy(tempptr - > zip, arr[i] - > zip);

                /* swaps values */
                strcpy(arr[i] - > name, arr[j] - > name);
                strcpy(arr[i] - > address, arr[j] - > address);
                strcpy(arr[i] - > citystate, arr[j] - > citystate);
                strcpy(arr[i] - > zip, arr[j] - > zip);

                strcpy(arr[j] - > name, tempptr - > name);
                strcpy(arr[j] - > address, tempptr - > address);
                strcpy(arr[j] - > citystate, tempptr - > citystate);
                strcpy(arr[j] - > zip, tempptr - > zip);
            }

        }
    }
}

I've included the printf statement to tell me how many times my program actually enters the loop. I am setting a variable equal to the value returned by strcmp, which would be positive if the zip code at index [i] was greater than the one in index [j] of the array. If this was the case, I would enter my sort and swap the values in the two indices, but my program never enters the loop. What am I doing wrong here?

Structure definition:

typedef struct person{
char name[50];
char address[50];
char citystate[30];
char zip[10];
}Person;

Function that allocates memory:

int takedata(Person *arr[])
{
/* counter variable */
int i = 0;
char teststring[25];

while ((gets(teststring)) != NULL && i < 50)
{
    /* dynamically allocates memory for each index of the array */
    arr[i] = (Person *)malloc(sizeof(Person));

    /* takes in data from user/ input file */

    strcpy(arr[i]->name, teststring);
    gets(arr[i]->address);
    gets(arr[i]->citystate);
    gets(arr[i]->zip);

    i++;    
}



    printf("Processed %d sets of data.\n\n", i);

    return (i-1);
}

sample data:

A1, A2
20294 Lorenzana Dr
Woodland Hills, CA
91364
B1, B2
19831 Henshaw St
Culver City, CA
94023
C1, C2
5142 Dumont Pl
Azusa, CA
91112
D1, D2
20636 De Forest St
Woodland Hills, CA
91364
A1, A2
20294 Lorenzana Dr
Woodland Hills, CA
91364
E1, E2
4851 Poe Ave
Woodland Hills, CA
91364
F1, F2
20225 Lorenzana Dr
Los Angeles, CA
91111
G1, G2
20253 Lorenzana Dr
Los Angeles, CA
90005
H1, H2
5241 Del Moreno Dr
Los Angeles, CA
91110
I1, I2
5332 Felice Pl
Stevenson Ranch, CA
94135
J1, J2
5135 Quakertown Ave
Thousand Oaks, CA
91362
K1, K2
720 Eucalyptus Ave 105
Inglewood, CA
89030
L1, L2
5021 Dumont Pl
Woodland Hills, CA
91364
M1, M2
4819 Quedo Pl
Westlake Village, CA
91362
I1, I2
5332 Felice Pl
Stevenson Ranch, CA
94135
I1, I2
5332 Felice Pl
Stevenson Ranch, CA
94135
N1, N2
20044 Wells Dr
Beverly Hills, CA
90210
O1, O2
7659 Mckinley Ave
Los Angeles, CA
90001

Answer 1

The main problem in your code is that you have a ; in the wrong place, I am sure by oversight.

for (i = 0; i <= (noElements - 1); i++); {
                                       ^^ Remove the ;

It ends the first for loop. That is messing up your logic.

Regarding the comment by @DavidHoelzer:

You can replace the lines:

strcpy(tempptr - > name, arr[i] - > name);
strcpy(tempptr - > address, arr[i]);
strcpy(tempptr - > citystate, arr[i] - > address);
strcpy(tempptr - > zip, arr[i] - > zip);

/* swaps values */
strcpy(arr[i] - > name, arr[j] - > name);
strcpy(arr[i] - > address, arr[j] - > address);
strcpy(arr[i] - > citystate, arr[j] - > citystate);
strcpy(arr[i] - > zip, arr[j] - > zip);

strcpy(arr[j] - > name, tempptr - > name);
strcpy(arr[j] - > address, tempptr - > address);
strcpy(arr[j] - > citystate, tempptr - > citystate);
strcpy(arr[j] - > zip, tempptr - > zip);

by

tempptr = arr[i];
arr[i] = arr[j];
arr[j] = tempptr;

where tempPtr is declared simply as:

Person * tempptr = NULL;

Caution

Use of gets is strongly discouraged. Read Why is the gets function so dangerous that it should not be used?.