user2309750
user2309750

Reputation: 1577

Can't get string search to work properly

I'm writing a simple program that gets user input from the console to calculate the resistance value of a resistor, and part of it involves comparing user input to a list of predetermined strings.

I've gotten most of it figured out (at least I believe so), but whenever I call the "search" function, which is supposed to return the index of the array of strings that matches the input and -1 otherwise. Problem is, whenever I call it in the program proper it always returns -1, and thus the program prints "invalid color" even when it isn't. I'm not sure where the problem lies, and I'd like to know what exactly I can do about it.

This is the program in question:

#include <stdio.h>
#include <math.h>
#include <string.h>

char COLOR_CODES[10][7] = {"black", "brown", "red", "orange", "yellow", "green", "blue", "violet", "gray", "white"};
int COMPARE_LIMIT = 7;

int search(char array[10][7], int size, char target[7]);

int main(void)
{
  //used for repeating input menu
  int menu = 0;
  char menuChoice = 0;

  //string variables hold input colors.
  char band1[7];
  char band2[7];
  char band3[7];

  //int variables hold int values of subscript for bands 1-3
  int value1;
  int value2;
  int value3;

  //value of resistor in ohmn
  int ohms;

  //holds value of multiplier for third band
  int powerMultiplier;

  while(menu == 0)
  {
    printf("Enter the colors of the resistor's three bands,\n");
    printf("beginning with the band nearest the end. Type\n");
    printf("the colors in lowercase letters only. NO CAPS\n");
    printf("OR SPACES.\n");

    printf("Band 1:");
    scanf("%s", &band1);
    printf("Band 2:");
    scanf("%s", &band2);
    printf("Band 3:");
    scanf("%s", &band3);

    if(search(COLOR_CODES, 10, band1) == -1)
    {
      printf("Invalid Color: %s\n", band1);
    }

    else if(search(COLOR_CODES, 10, band2) == -1)
    {
      printf("Invalid Color: %n\n", band2);
    }

    else if(search(COLOR_CODES, 10, band3) == -1)
    {
      printf("Invalid Color: %s\n", band3);
    }

    else
    {
      value1 = search(COLOR_CODES, 10, band1);
      value2 = search(COLOR_CODES, 10, band2);
      value3 = search(COLOR_CODES, 10, band3);

      powerMultiplier = (int)pow(10, value3);

      ohms = (value1 * 10) + value2;
      ohms = ohms * powerMultiplier;

      if(ohms < 1000)
      {
        printf("Resistance value: %s ohms", ohms);
      }

      else
      {
        int kiloOhms = (ohms / 1000);
        printf("Resistance value: %s kilo-ohms", kiloOhms);
      }

      printf("Do you want to decode another resistor? (y/n)");
      scanf("%c", &menuChoice);

      while(menuChoice != 'y' && menuChoice != 'Y' && menuChoice != 'n' && menuChoice != 'N')
      {
        if(menuChoice != 'Y' && menuChoice != 'y')
        {
          if(menuChoice == 'N' || menuChoice == 'n')
          {
            menu = 0;
          }

          else
          {
            printf("Invalid choice.");
          }
        }
      }
    }
  }

  return 0;
}

//returns the subscript of array[][] that matches target[].
int search(char array[10][7], int size, char target[7])
{
  int n;
  for (n = 0; n < size; n++)
  {
    if (strncmp(array[n], target, COMPARE_LIMIT) == 0)
    {
      return n;
    }

    else
    {
      return -1;
    }
  }
}

Upvotes: 0

Views: 612

Answers (3)

md_rasler
md_rasler

Reputation: 386

Greg has been kind enough to address the specific question asked, the search function is returning during the first iteration of the for loop.

I think it is also worth mentioning to not ignore compiler warnings, especially as you are learning a new language. Here is what the compiler is saying about the code sample that you submitted (in a file called tutorial2.c):

cc tutorial2.c -o tutorial2

tutorial2.c:40:17: warning: format specifies type 'char *' but the argument has type 'char (*)[7]' [-Wformat]
    scanf("%s", &band1);
           ~~   ^~~~~~
tutorial2.c:42:17: warning: format specifies type 'char *' but the argument has type 'char (*)[7]' [-Wformat]
    scanf("%s", &band2);
           ~~   ^~~~~~
tutorial2.c:44:17: warning: format specifies type 'char *' but the argument has type 'char (*)[7]' [-Wformat]
    scanf("%s", &band3);
           ~~   ^~~~~~
tutorial2.c:53:37: warning: format specifies type 'int *' but the argument has type 'char *' [-Wformat]
      printf("Invalid Color: %n\n", band2);
                             ~~     ^~~~~
tutorial2.c:74:45: warning: format specifies type 'char *' but the argument has type 'int' [-Wformat]
        printf("Resistance value: %s ohms", ohms);
                                  ~~        ^~~~
                                  %d
tutorial2.c:80:50: warning: format specifies type 'char *' but the argument has type 'int' [-Wformat]
        printf("Resistance value: %s kilo-ohms", kiloOhms);
                                  ~~             ^~~~~~~~
                                  %d
tutorial2.c:123:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^
7 warnings generated.

[Finished in 0.1s]

The line numbers will correspond to the line numbers in the code sample submitted. Note that the final warning is telling you that there is some aberrant behavior in the search function: that control has the ability to fall off the end of the function because there is a situation in which the "return" will never get hit. This should cue you into stepping through the for loop step by step. Also, the other warnings are worth looking into resolving.

Upvotes: 1

qdd
qdd

Reputation: 61

You should put the sentence "return -1;" in the last.Like this.

int search(char array[10][7], int size, char target[7])
{
    int n;
    for (n = 0; n < size; n++)
    {
        if (strncmp(array[n], target, COMPARE_LIMIT) == 0)
        {
            return n;
        }
    }
    return -1;
}

In addition,there are several errors. This one is very important that ohms and kiloOhms should be long long ,int is too short. This sentence:

else if(search(COLOR_CODES, 10, band2) == -1)
{
    printf("Invalid Color: %n\n", band2);
}

It should be %s.

And here,

if(ohms < 1000)
{
    printf("Resistance value: %s ohms", ohms);
}

    else
{
    int kiloOhms = (ohms / 1000);
    printf("Resistance value: %s kilo-ohms", kiloOhms);
}

It should be %d.

Upvotes: 0

Greg Hewgill
Greg Hewgill

Reputation: 993333

Your search() function returns -1 after looking at only the first item in the list. So it will probably find black but won't find anything else. Suggest modifying it as follows:

int search(char array[10][7], int size, char target[7])
{
  int n;
  for (n = 0; n < size; n++)
  {
    if (strncmp(array[n], target, COMPARE_LIMIT) == 0)
    {
      return n;
    }
  }
  return -1;
}

That way, the for loop will run to completion before it can decide that what you typed was not found, and so return -1.

Upvotes: 1

Related Questions