C Program Using Structures To Re-arrange List

Question

First C programming class and beginning to learn programming. I am currently learning how to use structures in C and this is a learning task to help understand this process.

The program should accept an input of addresses and organize them by zipcode from lowest zipcode to highest zipcode.

The program and input/output below is one of the programs I am using to learn this subject. I would like to dump the data into a file using file redirection with this program. For example:

ConsoleApplication1.exe < input.txt > testout.txt

Right now all that gets dumped into the test file is:

Processed 18 sets of data.

Obviously because of the printf statement I have in the 'takedate' function to test the process.

My question is, what would be the best way to take the input, process it, then dump the output in zipcode order into the file? Can anyone kindly recommend a function/statement that can be added in order to achieve this?

Thank you so very much in advance for your help, time, and guidance in getting this program working!

//header file initiating other headers
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

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

int takedata (Person * arr[]);
void sortdata (Person * arr[], int noElements);

int main (void) {

    //Intiate program
    int noElements = 0;
    Person *arr[50];

    //call input function
    //input(work, &count);
    takedata (arr);

    //sort files
    //call function
    //swap(work, &count);
    sortdata (arr, &noElements);

    //call output function
    //output(work, &count);

    //end program
    return 0;
}

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;
    Person *tempptr = NULL;

    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  */
                tempptr = arr[i];
                arr[i] = arr[j];
                arr[j] = tempptr;
            }
        }
    }
}

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);
}

Input data being used:

A1, A220294 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

As noted in the comments, there are a large number of errors in your code. From a security standpoint, never, never use gets. It was removed from the C-standard in C11 without deprecation given the security risks posed.

While not an error, the standard coding style for C avoids CamelCase variables in favor of all lower-case. See e.g. NASA - C Style Guide, 1994

In C, qsort is the defacto sorting routine provided by the library. Unless you just like to torment yourself, you should be using qsort to sort the array of pointers on zip and your sortdata routine can be removed entirely. The only thing you need to code for using qsort is an integer compare function so that qsort will know how to sort the pointers. Since you want to sort the list of pointers, you need to recognize that each input to the compare function will be a pointer to pointer to struct person, which means you will derefernce twice. A short compare function for qsort to sort on zip could be:

/** comparison function for qsort of pointers on zip */
int cmpzip (const void *a, const void *b)
{
    person *ap = *(person * const *)a;
    person *bp = *(person * const *)b;

    return strcmp (ap->zip, bp->zip);
}

(note: while the (person * const *) cast may look wonky, it is just a reflection that you are dealing with constant pointers whose values will not change. You could disregard the const modifier and write the cast as (person **) which visually makes more sense.)

Your entire sort of the array of pointers based on zip is reduced to:

    qsort (arr, nelem, sizeof *arr, cmpzip);

(using the sort routine provided by the C-library is much less error prone than rolling your own...)

Next, your code could fail in one of 50 different places and you would have no clue. You would just keep blindly reading and writing until a SEGFAULT (or other error) occurred. What if your call to malloc fails? Learn to validate each step that is relied on by a subsequent step, and validate all user input. (and I'll throw in the note about not casing malloc here -- it is wholly unnecessary) Examples of validation:

In main on your call to takedata:

    if (!(nelem = takedata (arr, fp))) { /* read addresses */
        fprintf (stderr, "error: no elements read.\n");
        return 1;
    }

In takedata validating allocation:

        /* allocate/validate memory for struct */
        if (!(arr[i] = malloc (sizeof *arr[i]))) {
            fprintf (stderr, "error: virtual memory exhausted.\n");
            exit (EXIT_FAILURE);
        }

Validating each read:

        /* read/validate address from stdin */
        if (!fgets (buf, MAXC, fp)) {
            fprintf (stderr, "error: read failure, arr[%d]->address.\n", i);
            exit (EXIT_FAILURE);
        }

Where possible add additional validations, like checking to insure your zip is all digits, e.g.

        if (!fgets (buf, MAXC, fp)) { /* zip */
            fprintf (stderr, "error: read failure, arr[%d]->zip.\n", i);
            exit (EXIT_FAILURE);
        }
        len = (size_t)rmcrlf (buf);  /* trims newline and returns length */
        char *p = buf;
        for (; *p; p++)
            if (*p < '0' || '9' < *p) { /* validate zip all numeric */
                fprintf (stderr, "error: invalid zip, arr[%d]->zip '%s'.\n",
                        i, buf);
                exit (EXIT_FAILURE);            
            }

There were a number of other issues I wanted to point out, but honestly, I lost track. Look over all the comments, they covered a bulk of the issues.

Lastly, putting all the pieces together in an example that reads your data in from a file (after fixing the first line), you could do something similar to the following. Make sure you understand what each line, each character, of the code is doing, if not, ask. The code will read your data from a file specified as the first argument on the command line (or from stdin by default if no argument is given).

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

/* constants for max zip, citystate, (structs, name, addr), buf */
enum { MAXZ = 10, MAXCS = 30, MAXS = 50, MAXC = 128 };

typedef struct person {
    char name[MAXS];
    char address[MAXS];
    char citystate[MAXCS];
    char zip[MAXZ];
} person;

size_t takedata (person **arr, FILE *fp);
int rmcrlf (char *s);
int cmpzip (const void *a, const void *b);

int main (int argc, char **argv) {

    size_t i, nelem = 0;    /* nelem cannot be negative */
    person *arr[MAXS];      /* C-style avoids mixed case */
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    if (!(nelem = takedata (arr, fp))) { /* read addresses */
        fprintf (stderr, "error: no elements read.\n");
        return 1;
    }
    if (fp != stdin) fclose (fp);   /* close file if not stdin */

    printf ("Processed %zu sets of data.\n\n", nelem);

    /* sort array of pointers on zip */
    qsort (arr, nelem, sizeof *arr, cmpzip);

    for (i = 0; i < nelem; i++) { /* print output free memory */
        printf (" name : %s  (%s)\n", arr[i]->name, arr[i]->zip);
        free (arr[i]);
    }

    return 0;
}

size_t takedata (person **arr, FILE *fp)
{
    if (!arr || !fp) {  /* validate arr and fp are non-NULL */
        fprintf (stderr, "takedata() error: invalid parameter.\n");
        return 0;
    }
    int i = 0;
    char buf[MAXC] = "";

    while (i < MAXS && fgets (buf, MAXC, fp)) {
        /* remove newline get length */
        size_t len = (size_t)rmcrlf (buf);

        /* allocate/validate memory for struct */
        if (!(arr[i] = malloc (sizeof *arr[i]))) {
            fprintf (stderr, "error: virtual memory exhausted.\n");
            exit (EXIT_FAILURE);
        }
        strncpy (arr[i]->name, buf, len + 1); /* copy buf to name */

        /* read/validate address from stdin */
        if (!fgets (buf, MAXC, fp)) {
            fprintf (stderr, "error: read failure, arr[%d]->address.\n", i);
            exit (EXIT_FAILURE);
        }
        len = (size_t)rmcrlf (buf);
        strncpy (arr[i]->address, buf, len + 1);

        if (!fgets (buf, MAXC, fp)) { /* citystate */
            fprintf (stderr, "error: read failure, arr[%d]->citystate.\n", i);
            exit (EXIT_FAILURE);
        }
        len = (size_t)rmcrlf (buf);
        strncpy (arr[i]->citystate, buf, len + 1);

        if (!fgets (buf, MAXC, fp)) { /* zip */
            fprintf (stderr, "error: read failure, arr[%d]->zip.\n", i);
            exit (EXIT_FAILURE);
        }
        len = (size_t)rmcrlf (buf);
        char *p = buf;
        for (; *p; p++)
            if (*p < '0' || '9' < *p) { /* validate zip all numeric */
                fprintf (stderr, "error: invalid zip, arr[%d]->zip '%s'.\n",
                        i, buf);
                exit (EXIT_FAILURE);            
            }
        strncpy (arr[i]->zip, buf, len + 1);

        i++;
    }

    return (i);
}

/** remove newline or carriage-return from 's'.
 *  returns new length, on success, -1 if 's' is NULL.
 */
int rmcrlf (char *s)
{
    if (!s) return -1;
    if (!*s) return 0;
    char *p = s;
    for (; *p && *p != '\n' && *p != '\r'; p++) {}
    *p = 0;

    return (int)(p - s);
}

/** comparison function for qsort of pointers on zip */
int cmpzip (const void *a, const void *b)
{
    person *ap = *(person * const *)a;
    person *bp = *(person * const *)b;

    return strcmp (ap->zip, bp->zip);
}

Input File

Your input with the first line fixed, e.g.

$ cat dat/addr.txt
A1, A2
20294 Lorenzana Dr
Woodland Hills, CA
91364
B1, B2
19831 Henshaw St
Culver City, CA
94023
C1, C2
...

Example Use/Output

Here is a quick print out of the sorted array of pointers showing the name and zip in sort order:

$ ./bin/addrstruct <dat/addr.txt
Processed 18 sets of data.

 name : K1, K2  (89030)
 name : O1, O2  (90001)
 name : G1, G2  (90005)
 name : N1, N2  (90210)
 name : H1, H2  (91110)
 name : F1, F2  (91111)
 name : C1, C2  (91112)
 name : J1, J2  (91362)
 name : M1, M2  (91362)
 name : A1, A2  (91364)
 name : D1, D2  (91364)
 name : A1, A2  (91364)
 name : E1, E2  (91364)
 name : L1, L2  (91364)
 name : B1, B2  (94023)
 name : I1, I2  (94135)
 name : I1, I2  (94135)
 name : I1, I2  (94135)

Memory Use/Error Check

In any code your write that dynamically allocates memory, you have 2 responsibilites regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to insure you haven't written beyond/outside your allocated block of memory, attempted to read or base a jump on an unintitialized value and finally to confirm that you have freed all the memory you have allocated.

$ valgrind ./bin/addrstruct <dat/addr.txt
==28762== Memcheck, a memory error detector
==28762== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==28762== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==28762== Command: ./bin/addrstruct
==28762==
Processed 18 sets of data.

 name : K1, K2  (89030)
 name : O1, O2  (90001)
 ...
 name : I1, I2  (94135)
 name : I1, I2  (94135)
==28762==
==28762== HEAP SUMMARY:
==28762==     in use at exit: 0 bytes in 0 blocks
==28762==   total heap usage: 18 allocs, 18 frees, 2,520 bytes allocated
==28762==
==28762== All heap blocks were freed -- no leaks are possible
==28762==
==28762== For counts of detected and suppressed errors, rerun with: -v
==28762== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)

Always confirm All heap blocks were freed -- no leaks are possible and equally important ERROR SUMMARY: 0 errors from 0 contexts.

There are always a lot of nuts & bolts of the language packed into the seemingly short, sort an array of pointers to struct, type examples. That's why professors love them -- and also why you should make sure you understand each piece of the puzzle. Look it over and let me know if you have any questions.

Fully Formatted Output

If you wanted to tweak the output and add each field of the struct and tidy up the formatting, you could do something similar to the following. Just replace the output in the code above with:

printf (" %-6s    %-22s    %-20s    %s\n", "Name",
        "Address", "City State", "Zip");
printf (" ------    ----------------------    "
        "--------------------    -----\n");
for (i = 0; i < nelem; i++) { /* print output free memory */
    printf (" %-6s    %-22s    %-20s    %s\n", arr[i]->name,
            arr[i]->address, arr[i]->citystate, arr[i]->zip);
    free (arr[i]);
}

For formatted output such as:

$ ./bin/addrstruct <dat/addr.txt

Processed 18 sets of data from 'stdin'.

 Name      Address                   City State              Zip
 ------    ----------------------    --------------------    -----
 K1, K2    720 Eucalyptus Ave 105    Inglewood, CA           89030
 O1, O2    7659 Mckinley Ave         Los Angeles, CA         90001
 G1, G2    20253 Lorenzana Dr        Los Angeles, CA         90005
 N1, N2    20044 Wells Dr            Beverly Hills, CA       90210
 ...

Answer 2

the function: main() has a variable noElements, initialized to 0

The function: takeData() keeps a local counter for the number of items read, but the value never gets back to the variable in main()

Then main() passes that value 0 to sortData() as the number of elements to sort. Naturally, given a count of 0, the sortData() function does nothing.

Why the variable: noElements having its' address passed to sortData()? The function: sortData() never changes that variable. so the code should simply pass the contents of noElements, Which, is what the function:sortData() is expecting, rather than an address.

no where in the posted code is the contents of the arr[] array of struct Person being displayed.