Determine if a word is an isogram, array subscription error

user_13960223 0
Viewed 73

I wrote a function that determines if a word is an isogram or not. I am close to the end but I have got few errors which I am not able to understand, because one of my compiler understands it but another doesn't.

The errors are:

solution.c:23:9: warning: array subscript is of type 'char' [-Wchar-subscripts]
        isogram[c]=isogram[c]+1;
               ^~
solution.c:23:20: warning: array subscript is of type 'char' [-Wchar-subscripts]
        isogram[c]=isogram[c]+1;
                      ^~
2 warnings generated.

And the code:

bool IsIsogram(char *str)
{
  int isogram[256];
  int i,len_str,cpt,occs;
  char c;

  for (i = 0; str[i] != '\0'; i++) {
      len_str=i;
  }

  for (i=0; i<=256;i++){isogram[i]=0;}
  for(i=0; i <= len_str; i++){
      c = str[i];
      c = tolower(c);
      str[i] = c;
  }
 cpt=0;
 for (i = 0; i < len_str; i++) {
    c = str[i];
    isogram[c]=isogram[c]+1;
  }

  for (i = 0; i < 256; i++) {
  occs = isogram[i];
  if (occs > 0) {
    cpt++;
  }
  }
  if (cpt==len_str) {
      return true;
  }
  else{
  return false;
  }
}
c
2 Answers

Type char is a bad decision to subscript an array. It can be signed or unsigned. You can fix the warning using unsigned char. You should also change the range of your for loop.

bool IsIsogram(char *str)
{
  int isogram[256];
  int i,len_str,cpt,occs;
  unsigned char c;

  for (i = 0; str[i] != '\0'; i++) {
      len_str=i;
  }

  for (i=0; i<256;i++){isogram[i]=0;} // i <=256 goes out of bounds
  for(i=0; i <= len_str; i++){
      c = str[i];
      c = tolower(c);
      str[i] = c;
  }
 cpt=0;
 for (i = 0; i < len_str; i++) {
    c = str[i];
    isogram[c]=isogram[c]+1;
  }

  for (i = 0; i < 256; i++) {
  occs = isogram[i];
  if (occs > 0) {
    cpt++;
  }
  }
  if (cpt==len_str) {
      return true;
  }
  else{
  return false;
  }
}
user_46453340

The warning is because char is usually a bad idea to use as subscript. If it's signed, the max value is 127.

Solution: Change char c; to unsigned char c;

Also, change for (i=0; i<=256;i++) to for (i=0; i<256;i++) or you will go out of bounds. You could do for (i=0; i<=255;i++) but it's ideomatic in C to loop over a half open interval.

That fixes the bugs. But also consider declaring loop variables in the for header instead of outside: for (int i=0; i<256;i++) but in this case, the easiest thing would be to skip the first loop completely that is only for zeroing the array, and instead initialize the array upon declaration: int isogram[256] = {0};

user_66994330
Related