String not printing fully C

Question

NOTE: This is for homework, I just want to know why i get the messed up print, not the finished program.

#include    <stdio.h>

char    get_band( int band_number, char band_color[] );
void    get_resistance( int resist, int power );

int main()
{
    int     resist;
    int     power;
    get_resistance(resist, power );
}

void get_resistance( int resist, int power )
{
    int     band_number;
    char    band_color[3];
    char    color[3];


    get_band( band_number, band_color );
    printf("%s", band_color);
}
char get_band( int band_number, char band_color[] )
{
    int     x;
    x=0;
    while (x < 3)
    {
        printf("Which band would you like to select? (1-3)\nDo not select one you have selected prior!\t");
        scanf("%d", &band_number);

        if (band_number = 1)
        {
            printf("What color would you like to assign here?\t");
            scanf("%s", &band_color[0]);
            x++;
        }
        else if (band_number = 2)
        {
            printf("What color would you like to assign here?\t");
            scanf("%s", &band_color[1]);
            x++;
        }
        else if (band_number = 3)
        {
            printf("What color would you like to assign here?\t");
            scanf("%s", &band_color[2]);
            x++;
        }

    }
    return (*band_color);

}

So when I run it, I get no errors or warnings, but what I do get is the last color I enter. For example, I enter green, blue, yellow, in that order. I will get yellow printed back. Regardless of what order I use for the numbers, I always get back the very last color entered.

Answer 1

You have two major problems with your code:

First:

if (band_number = 1) {
  ...
}

= is the assignment operator == is the equal to operator. Since band_number = 1 always evaluates to 1 which is true that branch is always taken.

Second:

char    band_color[3];

and then later:

scanf("%s", &band_color[0]);

You have not declared enough space and you stomping over your previous values. Furthermore you want to statically allocate an array of strings you need to do something more like the following:

void get_resistance( int resist, int power )
{
  int     band_number;
  char    band_color[3][20];


  get_band( band_number, band_color );
  printf("%s\n", band_color[0]);
  printf("%s\n", band_color[1]);
  printf("%s\n", band_color[2]);
}

void get_band( int band_number, char band_color[][20] )
{
  int     x;
  x=0;
  while (x < 3)
    {
      printf("Which band would you like to select? (1-3)\nDo not select one you have selected prior!\t");
      scanf("%d", &band_number);
      printf("What color would you like to assign here?\t");

      scanf("%s", band_color[band_number - 1]);
      x++;
    }

}

I have greatly simplified your code logic, as you had a lot duplication. The key point though is that char band_color[3][20] will declare enough space for 3 strings that are 20 characters long (including the terminating character). The function prototype to get_band has changed to accommodate the new type of band_color.

The code simplification actually removed the if statements by using the observation that the number inputted by the user was always 1 greater than the array index. You can then index the array with that number minus 1, i.e. band_color[band_number - 1].

Note this is susceptible to a buffer overflow if you type in a string that is two long when prompted. It is not high quality code, just trying to demonstrate your mistake

If might find this link helpful with regards to getting user input.

Also a good link to why using scanf is bad for user input.

Answer 2

There a few things wrong with your program. There is a lot of bad redundant passing of variables needlessly. You can either declare band_color as a global variable or declare it in get_band function. Same thing with band_number. There is no reason to declare it in get_resistance just to pass it to get_band. Just declare it in get_band as that is the only place it is used. You can then return a pointer to the band_color array to use for the printf. In your if statements in get_band you mis-wrote the evaluators. You used = where they should be ==.

if (band_number == 1)

This is causing your code to allows go into the first if and assign a variable only to the [0] array. Your array only needs to be [2] as that gives you 3 entries. Also you need to declare a length for the strings in your array with a second set of braces.

char band_color[2][10];