erykkk
erykkk

Reputation: 119

C - Incorrect result in counting number of empty elements in a 2D array

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:

Example from Command line

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.

Debugger varaibles

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

Answers (2)

Pablo
Pablo

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 songsArtists 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

hafeez
hafeez

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

Related Questions