CS50 Caesar Cipher giving incorrect output (pset2)

Question

I am trying to implement a program that encrypts messages using Caesar's cipher. I have some sort of logical error in my code, and I've been pulling my hair trying to find it. The logic makes sense to me, but I keep getting the wrong output. Please can someone guide me with this issue!

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

int main(int argc, string argv[])
{
    //Check that program was run with one command-line argument
    if (argc == 2)
    {
       int n = strlen(argv[1]);

       //Iterate over the provided argument to make sure all characters are digits

       for (int i = 0; i < n; i++) {

           if (isdigit(argv[1][i])) {
               //Convert that command-line argument from a string to an int
             //  int key = atoi(argv[1]);
                   int key = atoi(argv[1]);
                //Prompt user for plaintext
                string plaintext = get_string("Enter plain text: ");
                printf("ciphertext: ");

                //int l = strlen(plaintext);
                //Iterate over each character of the plaintext:
                for (int j = 0, l=strlen(plaintext); j < l; j++) {

                if (isalpha(plaintext[j])) {

                    if (isupper(plaintext[j])) {

                         printf("%c", (((plaintext[i] - 65) + key) % 26) + 65);
                    }
                    if (islower(plaintext[j])) {
                         printf("%c", (((plaintext[i] - 97) + key) % 26) + 97);
                    }

                  }
                  else
                    {
                         printf("%c", plaintext[i] );
                    }
                }
                printf("\n");
                return 0;
           }
           else {
                printf("Usage: ./caesar key\n");
                return 1;
           }
       }
    }
    else
    {
         printf("Usage: ./caesar key\n");
         return 1;
    }

}

Answer 1

The code that performs the encryption should not be inside the loop that checks that all the characters in argv[1] are digits. First do the loop that validates the key. If that succeeds, then ask the the plaintext and perform the encryption.

The major logical error is that you have plaintext[i] in a number of places. That should be plaintext[j].

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

int main(int argc, string argv[])
{
    //Check that program was run with one command-line argument
    if (argc == 2)
    {
        int n = strlen(argv[1]);

        //Iterate over the provided argument to make sure all characters are digits
        for (int i = 0; i < n; i++) {
            if (!isdigit(argv[1][i])) {
                printf("Error: Key must be numeric");
                return 1;
            }
        }

        //Convert that command-line argument from a string to an int
        int key = atoi(argv[1]);

        //Prompt user for plaintext
        string plaintext = get_string("Enter plain text: ");
        printf("ciphertext: ");

        //Iterate over each character of the plaintext:
        for (int j = 0, l=strlen(plaintext); j < l; j++) {
            if (isupper(plaintext[j])) {
                printf("%c", (((plaintext[j] - 'A') + key) % 26) + 'A');
            } else if (islower(plaintext[j])) {
                printf("%c", (((plaintext[j] - 'a') + key) % 26) + 'a');
            } else {
                printf("%c", plaintext[j] );
            }
        }
        printf("\n");
        return 0;
    } else {
        printf("Usage: ./caesar key\n");
        return 1;
    }
}

There's also no need to nest the isupper() and islower() checks inside isalpha(). Just use else if to test the 3 mutually exclusive conditions: uppercase letter, lowercase letter, everything else.

And avoid hard-coding ASCII codes, use character literals.