My program passes all the tests in the check50 except for the case dealing with Hax0r2 input i.e key word containing nonalphabetical characters. Check test results are included below.

:) vigenere.c exists
:) vigenere.c compiles
:) encrypts "a" as "a" using "a" as keyword
:) encrypts "world, say hello!" as "xoqmd, rby gflkp!" using "baz" as keyword
:) encrypts "BaRFoo" as "CaQGon" using "BaZ" as keyword
:) encrypts "BARFOO" as "CAQGON" using "BAZ" as keyword
:) handles lack of argv[1]
:) handles argc > 2
:( rejects "Hax0r2" as keyword
   \ expected output, not a prompt for input

As per Kareem's suggestion, I have added a do while loop at the beginning of this program. The do while loop is listed below.

//checking if keyword contains nonalphabetical characters
int i=0;
    if (isalpha(argv[1][i]!=0))
        printf( "%s","keyword contains nonalphabetical charaters");   
        return 1;
while (i< strlen(argv[1]));

While my other cases with checks on argc and argv[1] work, this loop does not appear to be working. The program appears to ignore the instruction if (isalpha(argv[1][i]!=0)) and prints the message"keyword contains nonalphabetical characters" whether the keyword contains nonalphabetical characters or not if I remove the return 1 command. If I include my return 1 command, the program will not start at all, i.e it exits at the starting itself because of return 1 command. Any suggestions on how to fix this last Hax0r2 case will be appreciated.


  • It means your condition is always returning true. – Mustaghees Aug 7 '15 at 5:56

Your program has a programming flaw.

You are checking if argv[1][i] != 0 while you should check for isalpha(argv[1][i]) != 0. Since argv[1] is an array of chars no index will be equal with 0. So this (argv[1][i] != 0) will always evaluate to true which is equal to a nonzero integer in C (say 1 for example).

Now if isalpha() gets as input a non alphabetical character, it returns a nonzero integer (say 1 for example again) which by it's turn evaluates to true in your condition, so it's always true.

So every time you run the program the flow goes into the if block prints your statement about non alphabetical characters and return 1 which terminates the program.

Even if you fix this though your code will still not work because of a logic flow. Consider the case where the input is correct (like hello). isalpha() will check the first char of argv[1] and it will return 0, so your if condition will be false the code inside will not execute. On the next iteration, i hasn't change since the i++ is inside the if block that didn't get executed. And this will invoke an infinite loop. You will have to solve this.


Try this as your if condition:

if ( !isalpha(argv[1][i]) )

You can improve efficiency of your program by first storing argv[1] and its length in a variable.

EDIT: This is how I implemented it.

 // get key and its length
char* key = argv[1];
int lenKey = strlen(key);

// verify that key is all alphabetic
for (int i = 0; i < lenKey); i++)
    if (!isalpha(key[i]))
        printf("There was a non-alphabetic character at %i\n", i);
        return 1;
  • -1. That will check the first character of each string in argv (and will possibly cause a segmentation fault). isalpha() requires as input a char, not a char* (i.e. a string) – ChrisG Aug 7 '15 at 6:49
  • I mean the if condition inside his while loop – Mustaghees Aug 7 '15 at 7:00
  • And so do I.... – ChrisG Aug 7 '15 at 7:20
  • See my implementation. Is it also wrong? It worked for me and I got 1/1 in pset2. – Mustaghees Aug 7 '15 at 7:41
  • Your implementation is correct indeed. But in your implementation you first assign argv[1] to key, and then you say isalpha(key[i]) which is the same as isalpha(argv[1][i]) not isalpha(argv[i]) as your answer suggests. – ChrisG Aug 7 '15 at 7:47

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .