1

I am having trouble figuring out how to check each character in the keyword (argv[1]). I know I am probably missing something super obvious. :(

I have tried saving the string to an array, declaring a new int, but still same problem.

//check to make sure 2nd argument is fully alphabetic
string keyword = argv[1];

for(int i = 0, n = strlen(keyword); i < n; i++)
{
    if(isalpha(keyword[i]))
    {
        printf("Success! \n");
        return 0;
    }
    else
    {
        printf("Invalid key, must be fully alphabetic. \n");
        return 1;
    }
}

Expected output should be "Invalid key, must be fully alphabetic." for anything not fully alphabetic. Instead, it only works for the beginning character, not the whole keyword.

alexas
  • 11
  • 4
  • 1
    That's because you `return` from inside the loop, after the first test. Pass or fail, that's all it does. – Weather Vane May 28 '19 at 22:34
  • `strlen()` has to iterate the string to find the end, but you then iterate the string in any case, it is simpler and more efficient to use the string terminator directly to terminate your loop: `for( int i = 0; keyword[i] != 0; i++ )` – Clifford May 28 '19 at 22:54
  • You might be interested in [cs50 stack exchange site](https://cs50.stackexchange.com). – pmg May 29 '19 at 11:59

2 Answers2

2

Don't short circuit (by returning) unless the value is non-alphabetic; save printing Success and returning 0 for when the whole loop completes without exiting due to non-alphabetic characters:

for(int i = 0, n = strlen(keyword); i < n; i++)
{
    if(!isalpha(keyword[i]))
    {
        printf("Invalid key, must be fully alphabetic. \n");
        return 1;
    }
}
printf("Success! \n");
return 0;
ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
  • Thank you. I had switch the print commands because I thought `!isalpha` wasn't executing properly for some reason, and figured I'd give it a try. What I am having real trouble with is iterating over the string to check that each character is alphabetic. I thought that was what `if(isalpha(keyword[i])` was doing, but it is not working. It is still saying `Success!` if it starts with an alphabetic character and continues on with a numeric character. – alexas May 29 '19 at 02:54
  • @alexa: Are you saying the code above is *still* misbehaving? If so, my only guess would be the use of a `string`, which isn't a thing in `C`. Declare `keyword` to be a `const char *` to keep it C. – ShadowRanger May 29 '19 at 10:44
  • Thank you! Yes, that was my issue. Oops. I understand now. It is working perfectly. :) – alexas May 29 '19 at 13:12
1

Two problems

Do not always exit the loop

@ShadowRanger

Use unsigned character values

isalpha(int x) is defined for x in the unsigned char range and EOF. Other negative char values re undefined behavior.

// if(!isalpha(keyword[i]))
if(!isalpha((unsigned char) keyword[i]))

With simplified loop - strlen() not needed.

int alexa_alpha_test(const char *keyword) {
  while (*keyword) {
    if(!isalpha((unsigned char) *keyword)) {
      printf("Invalid key, must be fully alphabetic. \n");
      return 1;
    }
    keyword++; 
  }
  printf("Success! \n");
  return 0;
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • 1
    *`isalpha(int x)` is defined for `x` in the `unsigned char` range and EOF. Other negative `char` values are undefined behavior.* And **another** reason to never, ever use CS50's misguided `typedef char *string;` bodge. – Andrew Henle May 29 '19 at 11:44
  • @AndrewHenle Yes the CS50 `string keyword` seems to perpetuate incorrect _string_ understanding as `keyword` is not a C string, but a pointer to (hopefully) a string - with IDB sign-ness characters. More of the usual problems of type-defining a pointer. – chux - Reinstate Monica May 29 '19 at 11:51
  • @chux Oh ok, thank you for explaining in detail! That makes much more sense. Works perfectly now. I appreciate the help and clarification. – alexas May 29 '19 at 13:07