Wsrtjtyk

In the following code I am trying for each band to get the number of band members. I've tried a number of things but nothing works. The following looks like it should but doesn't.

If any one could point out what I'm doing wrong it would be greatly appreciated.

numMembers = sizeof(bands[0]) / sizeof(bands[0].members);



#include <stdio.h>

#include <stdlib.h>

#include <string.h>



int main(void) {

    int         i;  

    int         j;  

    int         numBands;

    int         numMembers;

    int         limit = 4;



    struct band {

        char        name[10];

        char        *members[20];

    };  



    const struct band bands =

        {   {"Beatles", {"John", "George", "Paul", "Ringo", "Pete", "George"} },

            {"Stones",  {"Mick", "Keith", "Bill", "Charlie", "Brian"} },

            {"Who",     {"Pete", "Roger", "Keith", "John"} },

            {"JHE",     {"Jimmy", "Noel", "Mitch"} }  };  



    numBands   = sizeof(bands) / sizeof(bands[0]);



    for ( i = 0; i < numBands; ++i ) { 

        printf ("%sn", bands[i].name);

        numMembers = sizeof(bands[0]) / sizeof(bands[0].members);

        for ( j = 0; j < numMembers; ++j )

            printf ("t%s", bands[i].members[j]);

        printf ("n");

    }   



    return 0;

}

You don't have a "two dimensional array". What you have is a simple array of struct band which contains two simple character arrays as its members.

There is actually no need to compute the number of "members" in each bands. All you need to compute is the number of bands, e.g.

    int nbands = sizeof bands / sizeof *bands;

Since you have declared members as an array of pointers and have used an initializer to initialize the first four, the remaining pointers will have all bytes set 0 resulting in each being NULL. You can simply loop for the members with while (bands[i].members[j]), incrementing j each iteration, e.g.

#include <stdio.h>



struct band {

    char    name[10];

    char    *members[20];

};



int main(void) {



    const struct band bands = {

        {"Beatles", {"John", "George", "Paul", "Ringo", "Pete", "George"} },

        {"Stones",  {"Mick", "Keith", "Bill", "Charlie", "Brian"} },

        {"Who",     {"Pete", "Roger", "Keith", "John"} },

        {"JHE",     {"Jimmy", "Noel", "Mitch"} } };  



    int nbands = sizeof bands / sizeof *bands;



    for (int i = 0; i < nbands; i++) {

        int j = 0;

        puts (bands[i].name);

        while (j < 20 && bands[i].members[j])

            printf (" %s", bands[i].members[j++]);

        putchar ('n');

    }

}

(note addition of j < 20, which as pointed out by @chux in the comments, the test of while (bands[i].members[j]) alone could result in Undefined Behavior in the case where there were additional bands added bands after initialization and all 20 pointers of the members struct member were filled)

Example Use/Output

$ ./bin/bandmembers

Beatles

 John George Paul Ringo Pete George

Stones

 Mick Keith Bill Charlie Brian

Who

 Pete Roger Keith John

JHE

 Jimmy Noel Mitch

Additionally, don't use magic numbers in your code. Instead, If you need a constant, #define one (or more), e.g.

#define MAXNM 10

#define MAXMB 20

or use a global enum to do the same thing, e.g.

enum { MAXNM = 10, MAXMB = 20 };

That will allow you to eliminate the magic numbers, e.g.

struct band {

    char    name[MAXNM];

    char    *members[MAXMB];

};

This becomes very important for maintainability as your code length grows.

what I'm doing wrong

Wrong numMembers calculation

numMembers should be the number of elements in the array. (20)

Each bands[i].member has 20 elements given char *members[20]. Several elements are populated with pointers to string literals. Most elements remain 0 (NULL).

// numMembers = sizeof(bands[0]) / sizeof(bands[0].members);

numMembers    = sizeof(bands[0].members) / sizeof(bands[0].members[0]);

Attempting to print NULL as a string

printf ("t%s", bands[i].members[j]); not valid when bands[i].members[j] is NULL.

Not all bands[i].members[j] are set to a non-NULL value.

numBands = sizeof(bands) / sizeof(bands[0]);



for ( i = 0; i < numBands; ++i ) { 

  printf ("%sn", bands[i].name);

  numMembers = sizeof(bands[i].members) / sizeof(bands[i].members[0]);



  for ( j = 0; j < numMembers; ++j ) {

    if (bands[i].members[j]) {

      printf ("t%s", bands[i].members[j]);

    } 

  }



  printf ("n");

  }

} 

Deeper:

const struct band bands = { {"Beatles", ... forms bands and sizes bands based on the number of initializers.

char *members[20]; is a fixed size even if the initialization did not supply 20 strings. The first elements members[20] are initialized per the list, the remaining elements have a pointer value of 0.

For numMembers = sizeof(bands[0]) / sizeof(bands[0].members);

sizeofband(bands[0]) will give the size of one band struct - that is the 10 bytes for name, and then 20 * the size of a pointer.

sizeof(bands[0].members); will give 20*thesizeof a pointer

Not what you want, as sizeof doesn't take into consideration the content of whatever the pointer points to.

It also doesn't take into consideration that some of those members pointers will be NULL so you don't want to count them.

Perhaps better doing:

for ( j = 0; j < 20 &&  bands[i].members[j] != NULL; ++j )

        printf ("t%s", bands[i].members[j]);