Writing a sorting function to sort an array of pointers, pointing to structures

Question

I'm writing a function to sort an array of pointers, pointing to structures, based on the value of a zip code. I found a sort function online (it's my first time writing a sort function) and thought I would play around with it and see what happens and I keep getting the error "Array type 'char[7] is not assignable' and I'm not sure why. Any ideas?

Thank you.

struct personCatalog {
char name[50];
char address[50];
char cityState[50];
char zipCode[7];
} ;

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

void bubble_sort(struct personCatalog *arrayOfPointers[]){
    int num1 = 0;

    while (arrayOfPointers[num1] != NULL) {
    atoi(arrayOfPointers[num1++]->zipCode);
    }

        int progress = 0;

        do {
            int i;
            progress = 0;
            for (i = 0; i < num1 - 2; ++i) {
                if (arrayOfPointers[i]->zipCode > arrayOfPointers[i + 1]->zipCode) {
                    struct personCatalog  temp = *arrayOfPointers[i];
                    arrayOfPointers[i] = arrayOfPointers[i + 1];
                    arrayOfPointers[i + 1] = &temp;

                    progress = 1;
                }
            }
        } while (progress);
    }

Error I'm receiving

memcpy(temp, arrayOfPointers[i], sizeof(struct personCatalog)); the first argument is the culprit (in the other line the second, both times it's temp). You should pass a pointer: &temp. But you're memcpying the whole struct, you can have that easier, temp = *arrayOfPointers[i];, structs are assignable. In your original, you tried to assign to temp->zipCode (temp was a pointer then). That was what caused the first error. If you only want to swap the zipCodes, you must memcpy(&(temp.zipCode), &(arrayOfpointers[i].zipCode), sizeof temp.zipCode); etc. — Daniel Fischer
– Daniel Fischer, Commented Nov 25, 2012 at 5:13
Hey Daniel, I updated the code using your recommendations and I got an error I've never seen before. I'm trying to upload a screenshot of it. — user1681673
– user1681673, Commented Nov 25, 2012 at 21:46
The problem that immediately jumps out is that temp is a local variable, so &temp becomes a dangling pointer after the if finishes. With a struct personCatalog temp;, you need to swap the pointed-to values, so *arrayOfPointers[i] = *arrayOfPointers[i+1]; etc. But since you have an array of pointers, it's less work to just swap the pointers, so use a pointer as temporary variable, struct personCatalog *temp = arrayOfPointers[i]; ... arrayOfPointers[i+1] = temp;. — Daniel Fischer
– Daniel Fischer, Commented Nov 25, 2012 at 21:52
But, the message says the symbol is undefined for architecture x86_64, are you maybe using a library that is compiled for 32-bits? — Daniel Fischer
– Daniel Fischer, Commented Nov 25, 2012 at 21:57

rutgersmike · Accepted Answer · 2012-11-25 03:25:00Z

1

The C language doesn't inherently know how to assign 7 chars to 7 other chars. It only allows you to assign one primitive type to another at a time:

zipCode[0] = temp[0]
zipCode[1] = temp[1];
// etc.

To copy arrays in C which are contiguous, like zipCode, you can use memcpy:

memcpy(zipCode, temp, 7);

Also, it's possible that I misread your intent on my tiny screen, but you also shouldn't assign a struct pointer to zipCode either.

answered Nov 25, 2012 at 3:25

rutgersmike

1,2131 gold badge11 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

user1681673 Over a year ago

I tried to use memcpy and I got an error saying, "Unterminated function-like macro invocation." Do you know why? Thank you.

rutgersmike Over a year ago

Update your code and comments above to reflect the new error in context. You should also #include <string.h> for memcpy.

user1681673 Over a year ago

There's a new error. I updated the code. Thanks for the help.

rutgersmike Over a year ago

The memcpy destination and source arguments must be addresses, so you can't pass temp, you must pass &temp or temp->zipCode if you want to copy to/from the zipCode array.

Ricky Stewart · Accepted Answer · 2012-11-25 03:31:12Z

1

There are a couple problems with this code, but you're getting this error because you're trying to assign values to arrays in the highlighted lines; that's why it's complaining about assignment to array type char[7]. This doesn't make much sense, since you can't change the location of an array. You can either swap the bytes with a call to memcpy, or, perhaps more idiomatically, change the definition of struct personCatalog such that zipcode is a true char * rather than a char array.

answered Nov 25, 2012 at 3:31

Ricky Stewart

1,1521 gold badge8 silver badges18 bronze badges

4 Comments

user1681673 Over a year ago

I tried to use memcpy and I got an error saying, "Unterminated function-like macro invocation." Do you know why? Thank you.

Daniel Fischer Over a year ago

@user1681673 Apparently, memcpy is implemented as a macro in your implementation. How did you try to call it? Maybe you forgot a closing parenthesis?

user1681673 Over a year ago

Ah, I missed a closing parenthesis. Thanks.

user1681673 Over a year ago

There's a new error, I updated the code. Thanks for the help.

wildplasser · Accepted Answer · 2012-11-25 22:02:25Z

0

The array consists of pointers. So the temp thing you are using to swap them should also be a pointer.

    for (i = 0; i < num1 - 1; ++i) {
            if ( strcmp( arrayOfPointers[i]->zipCode
                       , arrayOfPointers[i + 1]->zipCode
                       ) > 1) {
                struct personCatalog  *temp ;
                temp = arrayOfPointers[i];
                arrayOfPointers[i] = arrayOfPointers[i + 1];
                arrayOfPointers[i + 1] = temp;

                progress = 1;
            }
        }

Also, the loop condition i < num1-2 was wrong. Should be num1 -1, IMO.

Update: it appears zipcode is a textstring, so I replaced ">" by "strcmp()"

edited Nov 25, 2012 at 22:02

answered Nov 25, 2012 at 21:53

wildplasser

44.5k9 gold badges72 silver badges116 bronze badges

6 Comments

user1681673 Over a year ago

Before the function I changed zip code to an int using atoi().

wildplasser Over a year ago

But you ignore the return value. The atoi() is effectively a no-op.

user1681673 Over a year ago

Oh, I guess I was confused on how it worked. What would be a proper implementation of it?

user1681673 Over a year ago

I'm curious because I'm trying to sort the zip codes based on the lowest value.

wildplasser Over a year ago

You could do two things: 1) store the int value for the zipcode inside the structure (ou'll need to add another element to the struct) and assign to that, or 2) keep the ">" comparison, but wrap the zipcode operands each into an atio() function call.

|

Collectives™ on Stack Overflow

Writing a sorting function to sort an array of pointers, pointing to structures

3 Answers 3

4 Comments

4 Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

4 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related