0

Hey guys this is my first post here and i was wondering if any of you can help me figure out how to sort array of pointers to structures. Here's my code and here's my assignment if anyone is interested https://i.sstatic.net/Z9uUO.png.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <math.h>
#include <stdlib.h>
#include <string.h>

#define MAX 50

struct address
{
   char name[50];
   char street[50];
   char citystate[50];
   char zip[20];
};

int main()
{
   struct address *ptr[50];
   struct address tptr;
   char buffer[80];
   int count = 0;

   for (int i = 0; i <MAX; i++)
   {
      ptr[i] = malloc(sizeof(struct address));

      if (gets(buffer)== NULL)
      {
         break;
      }
      else
      {
         strcpy((*ptr[i]).name, buffer);
         gets((*ptr[i]).street);
         gets((*ptr[i]).citystate);
         gets((*ptr[i]).zip);
         free(ptr[i]);
         count++;
      }
   }

   for (int x = 0; x<count; x++)
   {
      for (int y = 0; y<count - 1; y++)
      {
         if ((*ptr[y]).zip>(*ptr[y + 1]).zip)
         {
            tptr = ptr[y + 1];
            ptr[y + 1] = ptr[y];
            ptr[y] = tptr;
         }
      }
   }

   for (int i = 0; i < count; i++)
   {
      puts((*ptr[i]).name);
      puts((*ptr[i]).street);
      puts((*ptr[i]).citystate);
      puts((*ptr[i]).zip);
   }
}
12
  • 3
    cplusplus.com/reference/cstdlib/qsort Commented May 18, 2015 at 4:22
  • 2
    start by indenting your code properly so that people (including yourself) can read it! Commented May 18, 2015 at 4:23
  • actually, before that, post something that compiles. In the line gets(buffer), buffer has not been declared. (Of course, gets is a mistake in itself too). Commented May 18, 2015 at 4:24
  • sorry about the indentation and i did declare the buffer but i forgot to include it in here Commented May 18, 2015 at 4:28
  • you call free(ptr[i]); which frees that address and then you go on to try and work with the addresses after freeing them... Commented May 18, 2015 at 4:32

3 Answers 3

2

Problems I see in your code:

  1. You are using gets. See another SO post that addresses the poblem of using gets. Use fgets instead.

    if (fgets(buffer, sizeof(buffer), stdin)== NULL)
    
    fgets((*ptr[i]).street, sizeof((*ptr[i]).street), stdin);
    fgets((*ptr[i]).citystate, sizeof((*ptr[i]).citystate), stdin);
    fgets((*ptr[i]).zip, sizeof((*ptr[i]).zip), stdin);
    
  2. You are calling free on a pointer in the following line

    free(ptr[i]);
    

    and continue to use it later in the code. Remove that line. Add the code to free the allocated memory at the end of the function.

    for (int i = 0; i < count; i++)
    {
       free(ptr[i]);
    }
    
  3. You are assigning a struct address* to a variable of type struct address in the following line:

    tptr = ptr[y + 1];
    

    and you are assigning a struct address to a variable of type struct address* in the following line:

    ptr[y] = tptr;
    

    both of them can be fixed by changing the type of tptr to struct address*.

    struct address *tptr;
    
  4. The following code is not appropriate for comparing two strings:

     if ((*ptr[y]).zip>(*ptr[y + 1]).zip)
    

    it only compares two pointer values. Use

     if (strcmp((*ptr[y]).zip,(*ptr[y + 1]).zip) > 0)
    
Sign up to request clarification or add additional context in comments.

Comments

0
  1. Why do you want to free() the allocated memory once you fetch the data from the user?
  2. You should free() the allocated memory at the end of the program, once after the printing is done.
  3. struct address tptr; should be of type struct address *tptr; as you are assigning the value of the pointer.

Below are the changes: 1.

for (int i = 0; i <MAX; i++)
{
    ptr[i] = malloc(sizeof(struct address));

    if (gets(buffer)== NULL)
    {
        free(ptr[i]); /* free the memory if data not read */
        break;
    }
    else
    {
        strcpy((*ptr[i]).name, buffer);
        gets((*ptr[i]).street);
        gets((*ptr[i]).citystate);
        gets((*ptr[i]).zip);
        /* Do not free the mem here as you are bound to lose the data */
        count++;
    }
}

2.

for (int i = 0; i < count; i++)
{
    puts((*ptr[i]).name);
    puts((*ptr[i]).street);
    puts((*ptr[i]).citystate);
    puts((*ptr[i]).zip);
    free(ptr[i]); /* free the memory here */
}

PS : Using gets() is not a good idea. Check this Do not use gets for more info

Comments

0

First, when dealing with pointer to structs, proper member access is with the -> operator (e.g. ptr[i]->street). As others have pointed out, do NOT use gets. It is no longer part of the C library and was deprecated because it was insecure. Use fgets or getline instead.

Next, (and this is a matter of form) avoid hardcoded numbers in your code. Use #define to set your values. This allows easy adjustment in a single place if values change.

With that, you were not far off with your code. Making only those changes, deleting the unnecessary math.h and adding strcmp for your sort, you can sort your structures in ascending order by zip as follows:

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

#define MAX 50
#define MAXL 80
#define MAXZ 20

struct address {
    char name[MAX];
    char street[MAX];
    char citystate[MAX];
    char zip[MAXZ];
};

int main ()
{
    struct address *ptr[MAX];
    struct address *tptr;
    char buffer[MAXL];
    int count = 0;

    for (int i = 0; i < MAX; i++) {
        ptr[i] = malloc (sizeof (struct address));

        if (fgets (buffer, MAXL, stdin) == NULL) {
            break;
        } else {
            strncpy (ptr[i]->name, buffer, MAXL);
            ptr[i]->name[MAX - 1] = 0;
            fgets (ptr[i]->street, MAX, stdin);
            fgets (ptr[i]->citystate, MAX, stdin);
            fgets (ptr[i]->zip, MAXZ, stdin);
            count++;
        }
    }

    for (int x = 0; x < count; x++) {
        for (int y = 0; y < (count - 1); y++) {
            if (strcmp (ptr[y]->zip, ptr[y + 1]->zip) > 0) {
                tptr = ptr[y + 1];
                ptr[y + 1] = ptr[y];
                ptr[y] = tptr;
            }
        }
    }

    for (int i = 0; i < count; i++) {
        printf ("\n%s", ptr[i]->name);
        printf ("%s", ptr[i]->street);
        printf ("%s", ptr[i]->citystate);
        printf ("%s", ptr[i]->zip);
    }
}

Input

$ cat dat/sortaddress.txt
some name
my street
my city, my state
55512
another name
another street
another city, another state
44412

Use/Output

$ ./bin/struct_address_sort <dat/sortaddress.txt

another name
another street
another city, another state
44412

some name
my street
my city, my state
55512

note: reading with fgets or getline will read the trailing newline and include that in the buffer. It is a good idea to strip the newline from your strings so you don't have miscellaneous newlines at the end of your data. There are many examples on StackOverflow.

2 Comments

Thank you very much for your help, the changes you suggested did the trick thank you.
Glad to help. Like I said, you were not far off, just needed a little help handling the structure syntax and strcmp for the comparison with sort. You can use qsort (recommended for any significant sorting), but for simple examples, a manual sort is fine.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.