PSET2, Caesar 2019: Validating Key

Question

I've written up my code for Caesar and for the most part I'm happy with it. If I start the program with ./caesar 10, it registers 10 as the key, and "encrypts" the plaintext letters by 10.

However, it appears that the for loop which is being used to iterate over argv[1] to detect letters, isn't actually iterating through argv[1]? Rather it's only looking at the first character? I've come to this conclusion based on the fact that ./caesar x3 would not start the program, and would return the error message; whereas ./caesar 3x will start the program successfully (which obviously it shouldn't.

Therefore, does anyone have any pointers as to why this doesn't appear to be going through each char in the argv[1] string? Or if I'm missing something else entirely?

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

int main(int argc, string argv[])
{
    //checking the two inputs ha
    if (argc == 2)
    {
        int n = strlen(argv[1]);
        int i;
        for (i = 0; i < n; i++) 
        {
            //checking argv[1] for any alphabetic characters present, if so                  returning the error message
            if (isalpha(argv[1][i]))    
            {
                printf("Usage: ./caesar key\n");
                return 1;    
            }
            //checking argv[1] for all decimal digits before converting the                 string to an interger
            else if (isdigit(argv[1][i]))
            {
                //this turns the key into an int variable
                int key = atoi(argv[1]);
                //requests the message to be coded, from the user
                string plaintext = get_string("Plaintext: ");
                printf("Ciphertext:");
                for (int b = 0, c = strlen(plaintext); b < c; b++)
                {
                    //checks if the character is upper case and if so encrypts it in upper case
                    if (isupper(plaintext[b]))
                    {
                        printf("%c", (((plaintext[b] + key) - 65) % 26) + 65);
                    }
                    //checks if character is lower case and if so encrypts it in lower case
                    else if (islower(plaintext[b]))
                    {    
                        printf("%c", (((plaintext[b] + key) - 97) % 26) + 97);
                    }
                    //if chracter is not upper or lower case, e.g. a question mark, it prints as a question mark
                    else
                    {
                        printf("%c", plaintext[b]);
                    }
                }
                return 0;
            }
        }
    }
    //if argc doesn't equal 2, printing error message back to user
    else
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
}

Answer 1

The code is trying to do too many things at once. It is trying to validate the key and encode the plaintext in the same loop. Can't do that.

You need to break the tasks down into separate, independent tasks.

First task is to validate the key. If it isn't a valid key, then the program should exit. The code is looking for alphas to validate the key. What if argv[1] contained a special char like $? Would that be valid? There are lots of character types.

Think about this. Sometimes, instead of asking if something IS something, you should ask if something IS NOT something. So, to start, maybe a loop that cycles through all chars in the key and see if anything IS NOT a digit? If that condition is true, then the key is invalid and the program should exit.

ONLY AFTER the key has been checked for non-digits, the key should be converted to an actual integer. Hint: atoi() is your friend.

Now, only after the key has been validated and stored in an int var, it's time to start encoding the plaintext. Maybe time for a new loop?

Oh, remember that a return 0; statement will terminate the entire program, not just the current loop.

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