Reputation: 119
I am creating a program for a college assignment where the user is required to input artists and their songs. The program then sorts them alphabetically and shuffles them. Artist names are stored in an array called artists[][80]
and song names are stored in songsArtistx
, where x is a number from 1 to 4. I initialised all arrays to be filled with the NULL terminator - '\0'. For the program to work, I need to find the number of songs entered (have to be at least 1, but can be 3 or less). To achieve this, I am using a function called checkSongs
:
int checkSongs(char songsOfAnArtist[][80])
{
int i,numOfSongs;
//Loop goes through 4 possible artists.
for (i=0;i<4;i++)
{
//Assume there are 3 songs for each artits, and decrement by 1 each time an empty string occurs.
numOfSongs = 3;
if (songsOfAnArtist[i][0]=='\0' || songsOfAnArtist [i][0] == '\n')
{
numOfSongs--;
break;
}
}
return numOfSongs;
}
However, this function gives me a faulty result for when the number of songs is less than 3. Here is an example from the command line, and also a screenshot of the variables from the debugger:
In the photo above, the numbers on the last line indicates the number of artists inputted (which is correct in this case) and the number of songs in songsArtsist1
, songsArtsist2
, songsArtsist3
, songsArtsist4
respectively. The last number is the number of artists again.
How do I alter my code so that checkSongs
returns the number of songs entered for each artists?
Below is also an excerpt from the main file which could be relevant to the question:
//Get all inputs from command line: artists and songs
getInputs(artists,songsArtist1,songsArtist2,songsArtist3,songsArtist4);
//Use checkArtists to store the number of entered artists in variable 'numOfArtists'
numOfArtists = checkArtists(artists);
printf("%d ",numOfArtists);
//Use check songs to store number of songs per artist in array 'numSongsPerArtists'
numSongsPerArtist[0] = checkSongs(songsArtist1);
numSongsPerArtist[1] = checkSongs(songsArtist2);
numSongsPerArtist[2] = checkSongs(songsArtist3);
numSongsPerArtist[3] = checkSongs(songsArtist4);
//DEBUG
printf("%d ",numSongsPerArtist[0]);
printf("%d ",numSongsPerArtist[1]);
printf("%d ",numSongsPerArtist[2]);
printf("%d ",numSongsPerArtist[3]);
printf("%d ",numOfArtists);
Here are there arrays:
//The array containing artists names
char artists[4][80];
//The array containing the sorted artists
char sortedArtists[4][80];
//Songs for Artist 1
char songsArtist1[3][80];
//Songs for Artist 2
char songsArtist2[3][80];
//Songs for Artist 3
char songsArtist3[3][80];
//Songs for Artist 4
char songsArtist4[3][80];
//The total number of artists (Note it can be less than 4)
int numOfArtists = 0;
//The total number of songs for each artist (Note that less than 3 songs can be provided for each artist)
int numSongsPerArtist[4] = {0,0,0,0};
Upvotes: 3
Views: 273
Reputation: 13590
When you write a function that takes an array as argument, it always should ask
for the length from the caller, unless the end of the array is marked somehow
(like '\0
' for strings). If you later change you program to accepts more or
less number of songs and you forget to update the loop conditions, you are in a
world of trouble. The caller knows the size of the array, either because it
created the array or because the array along with it's size was passed. This is
standard behaviour of the functions in the standard C library.
So I'd rewrite your functions as:
int checkSongs(char songsOfAnArtist[][80], size_t len)
{
int numOfSongs = 0;
for(size_t i = 0; i < len; ++i)
{
if(songsOfAnArtist[i][0] != 0 && songsOfAnArtist[i][0] != '\n')
numOfSongs++;
}
return numOfSongs;
}
And then calling the function
numSongsPerArtist[0] = checkSongs(songsArtist1, sizeof songsArtist1 / sizeof *songsArtist1);
numSongsPerArtist[1] = checkSongs(songsArtist2, sizeof songsArtist2 / sizeof *songsArtist2);
numSongsPerArtist[2] = checkSongs(songsArtist3, sizeof songsArtist3 / sizeof *songsArtist3);
numSongsPerArtist[3] = checkSongs(songsArtist4, sizeof songsArtist4 / sizeof *songsArtist4);
This is better because if you later change from char songsArtist3[3][80];
to
char songsArtist3[5][80];
, you don't have to rewrite the boundaries in the
loop conditions. Every artists can have a different size of slots for the songs.
And when you store the song names, make sure that the source string is not
longer than 79 characters long, use for example strncpy
and make sure to write the '\0'
-terminating byte.
If you keep having the wrong results, then it may be that the songsArtist
s
variables are not initialized correctly, please check your getInputs
function
so that all songsArtist[i][0]
are set to 0 on initialization. If you do
that, you should get the correct results.
For example:
int main(void)
{
...
char songsArtist1[3][80];
char songsArtist2[3][80];
char songsArtist3[3][80];
char songsArtist4[3][80];
memset(songsArtist1, 0, sizeof songsArtist1);
memset(songsArtist2, 0, sizeof songsArtist2);
memset(songsArtist3, 0, sizeof songsArtist3);
memset(songsArtist4, 0, sizeof songsArtist4);
...
}
Upvotes: 1
Reputation: 169
Add a condition in the block where you break the loop.
if(sumOfSongs==0)
break;
Also I would recommend to use unsigned
types for the variables as the numbers most likely can never be less than 0;
And put numOfSongs = 3;
outside the for
loop as others suggested.
Upvotes: 0