not satisfying the condition for input

Question

In command line argument,it should not accept argv[1] as a numeric or any other.It should accept only alphabetic and that argv[1] is treated as key which is to be added with plaintext.so,for that i have written a condition if(!alpha()).But still it is accepting argv[1] as nummeric.Please help me out with this.

    #include <stdio.h>
    #include <cs50.h>
    #include <string.h>
    #include <stdlib.h>
    #include <ctype.h>

    int main(int argc,string argv[])
    {

    if(argc==2)
    {   //to get input from user
    printf("plaintext: ");
    string s=get_string();
    //to print output
    printf("ciphertext: ");
    //to read plaintext till end
    for(int i=0,l=strlen(s);i<l;i++)
    {  //to assign argv[1] to key
    string key=argv[1];
    if(!isalpha(key[i]))
    {
    printf("Usage: ./vigenere k\n");
    return 1;
    }
    //to check if it is alphabetical
    else  if(isalpha(s[i])&&isalpha(key[i]))
     {
         for(int j=0,n=strlen(key);j<n;j++)
          {
           if(islower(s[i])&&islower(key[j]))
           {
            s[i]=(((s[i]-97)+((tolower(key[j])-97)%26))%26)+97;
           }
           else if(isupper(s[i])&&isupper(key[j]))
           {
            s[i]=(((s[i]-65)+((toupper(key[j])-65)%26))%26)+65;
           }
          }//for closed
        printf("%c",s[i]);
     }//else if closed
    }//for closed
    printf("\n");
    return 0;
    }//if closed
    else if(argc!=2)
    {
    printf("Usage: ./vigenere k\n");
    return 1;
    }
    }//main closed

Answer 1

The code is actually testing for non-alpha keys. The problem is that it is prompting for the plaintext before checking the key. It's a sequence issue.

As a side issue, consider how you might make the code more efficient. For example, the code is structured like this:

if(argc==2)
{
    //the bulk of the program
}

else if(argc!=2)
{
    printf("Usage: ./vigenere k\n");
    return 1;
}

Structuring a program like this makes the code vulnerable to bugs being introduced if the code is later modified. When code maintenance is done later, this structure isn't obvious and code could be placed somewhere so that it doesn't interact correctly with the if/else if structure. Also, the else if could be a simple else, but then, if done right, the only thing needed is an if.

Best practice would be to do the test at the beginning of the program with a single if statement. If the IF statement fails, the code continues no matter what, so an else clause is not needed.

if(argc!=2)
{
    printf("Usage: ./vigenere k\n");
    return 1;
}

By doing it like this, the test is executed, and the rest of the code is not wrapped in an if or else/else if clause and is independent of this code. It's much cleaner and simpler.

Further, the test for an all alpha key should immediately follow this test. If it fails, the code exits. BTW, the message should say "key must be alpha", not usage.

Also, once the key is verified as all alpha, the later test for isalpha(key[]) is not needed, simplifying the code even more.

Finally, the use of "magic numbers" 65 and 97 should be replaced by 'a' and 'A'. The use of the letters instead of the numbers explains to anyone reading or modifying the code later what those values really are and gives insight into why they're used.

There may be other issues, but this answers your specific question.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)