0

I have come up with the following code to solve caesar.c of pset2. It passes all check50 tests. However, it fails to detect when a string input is not a number. The way I understand it, I have to create a function or a for loop to check for it. Is there a shorter method?

I also had to use four if else in the ciphertext function. Is there any way I can shorten code there as well?

string ciphertext(string word, int key);

int main(int argc, string argv[])
{
    if (argv[1] == 0 || argv[2] != 0)
    {
        // Return error if the comma nd line argument is incorrect
        printf("Argument is incorrect. Usage: ./caesar key(integer)\n");
        exit(1);
    }

    // Get input words
    string word = get_string("plaintext: ");

    // Get command line parameter from argv[1]
    int p2 = atoi(argv[1]);
    if (p2 > 26)
    {
        p2 = p2 % 26;
    }

    // cipher words using ciphertext() function
    string cipher = ciphertext(word, p2);

    // Print output
    printf("ciphertext: %s\n", cipher);
}

string ciphertext(string word, int key)
{
    // Calculate ciphertext
    for (int i = 0, n =  strlen(word); i < n; i++)
    {
        char c = word[i];
        if (isupper(c) && (int) c + key <= 90) // For uppercase
        {
            if (isalpha(c))
            {
                c = (int) c + key;
                word[i] = c;
            }
        }
        else if (isupper(c) && (int) c + key > 90)
        {
            if (isalpha(c))
            {
                c = (int) c + key - 26;
                word[i] = c;
            }
        }
        else if (islower(c) && (int) c + key <= 122) // For lowercase
        {
            if (isalpha(c))
            {
                c = (int) c + key;
                word[i] = c;
            }
        }
        else if (islower(c) && (int) c + key > 122)
        {
            if (isalpha(c))
            {
                c = (int) c + key - 26;
                word[i] = c;
            }
        }
    }
    return word;
}
1

Instead of checking isalpha for each different condition, you could start with isalpha and then only move to the conditions if that is true. Otherwise, it would skip the character.

When working with loops, it always helpful to start with the most generic case first, and then nest the more specific ones to save on time; if the generic case is false, then none of the specific ones can be true.

1

Along with what Robert has said, you can use a small and simple for loop containing an if statement checking for a numeric character in the key using isdigit. If it is not a numeric character, then it returns 1. You can read about isdigit in the cs50 manual.

Hope this helps. If it does then please check the tickmark.

3
  • For the problem specifications, though, all non-alphanumeric characters should remain unchanged in the output. So, the loop should not return 1, unless it is in another function, as it will cause the program to fail the checks. – Robert S. Pratt Jan 31 at 16:33
  • 1
    @Robert, This answer was for checking the key – Vsjain Jan 31 at 17:05
  • I'm sorry; I either misread or didn't understand yours was for the key and not regarding the same application as mine. – Robert S. Pratt Jan 31 at 17:20

You must log in to answer this question.

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