views:

186

answers:

5

When trying to compile the following code, I am getting a warning that line 18 makes integer from pointer without cast and that 19 and 20 are incompatible types in assignment. I am new to structures in C, and can't seem to figure out what is wrong.

#include <stdio.h>

struct song
{       char title[70];
};

struct playlist
{       struct song songs[100];
};

void title_sort(struct playlist * list,int len)
{       int swapped = 1,i;
        char hold;
        while (swapped)
        {       swapped = 0;
                for (i = 0;i < len - 1; i++)
                {       if (list->songs[i].title > list->songs[i+1].title)
                        {       hold = list->songs[i].title;
                                list->songs[i].title = list->songs[i+1].title;
                                list->songs[i+1].title = hold;
                                swapped = 1;
                        }
                }
        }
}

int main()
{       struct playlist playlist;
        int i;
        for (i = 0;i < 5;i++)
        {       fgets(playlist.songs[i].title,70,stdin);
        }
        title_sort(&playlist,5);
        printf("\n");
        for (i = 0;i < 5;i++)
        {       printf("%s",playlist.songs[i].title);
        }
        return 0;
}
+7  A: 

You can't compare strings in C with >. You need to use strcmp. Also hold is char but title is char [70]. You could copy pointers to strings but arrays can't be copied with just =.

You could use strcpy like this:

void title_sort(struct playlist * list,int len)
{       int swapped = 1,i;
        char hold[70];
        while (swapped)
        {       swapped = 0;
                for (i = 0;i < len - 1; i++)
                    {       if (strcmp (list->songs[i].title, list->songs[i+1].title) > 0)
                            {       strcpy (hold, list->songs[i].title);
                                strcpy (list->songs[i].title, list->songs[i+1].title);
                                strcpy (list->songs[i+1].title,hold);
                                swapped = 1;
                        }
                }
        }
}

But please note that in C you need to check things like the lengths of strings, so the above code is dangerous. You need to either use strncpy or use strlen to check the lengths of the strings.

Kinopiko
+1  A: 

You can not use strings like that C. Strings are essentially a simple array of characters in C without specialized operators like =, < etc. You need to use string functions like strcmp and strcpy to do the string manipulations.

Naveen
+1  A: 

To be more specific : following is wrong

if (list->songs[i].title > list->songs[i+1].title)

Do it this way:

if( strcmp (list->songs[i].title , list->songs[i+1].title) > 0 )
Ravi
You're missing a `> 0)` at the end of your snippet there.
Adam Rosenfield
Oh. Thanks.Fixed!
Ravi
By the way, how can u blockquote something in a comment?Please teach me..
Ravi
+1  A: 
  • char hold needs to be something else, perhaps char *hold, perhaps an array.
  • C doesn't have array assignment, although it does have structure assignment, you will need to rework that
DigitalRoss
+1  A: 

Your first issue, on line 18, is caused by two problems. Firstly, the variable hold can only hold a single char value, and you're trying to assign an array of 70 chars to it.

First you'll need to make hold the correct type:

char hold[70];

Now, there's another problem - arrays can't just be assigned using the = operator. You have to use a function to explicitly copy the data from one array to another. Instead of your current line 18, you could use:

memcpy(hold, list->songs[i].title, 70);

You then need to do the same thing for lines 19 and 20:

memcpy(list->songs[i].title, list->songs[i+1].title, 70);
memcpy(list->songs[i+1].title, hold, 70);

Alternatively, you could write a loop and swap the two titles one char at a time.

In a similar fashion, you can't compare two strings with the simple < operator - you need to use a function for this, too (eg. strcmp).

caf