1

Hi guys, I'm struggling with my caesar program. I believe I've touched on every specs requirements, but the expected result isn't here. For instance, using a key of 1, "ABC" becomes "C". Please see my code below: (all the # libraries are there, I just won't copy them here)

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

// declare k and convert it to an integer
int k = atoi(argv[1]);

// making sure the command line strictly has 2 elements
if (argc == 2)
{

    // get user input
    string p = get_string("plaintext: ");
    string c = p;

    // encrypt one character at a time
    for (int i = 0, n = strlen(p); i < n; i++)
    {
        // check if string is letters or not
        if (isalpha(p[i])) // = if letters
        {
            if (isupper(p[i]))
            {
                // typecast p[i], index as uppercase letter and encrypt with k
                c[i] = (((int)p[i] - 65) + k) % 26;
            }
            else
            {
                (islower(p[i]));
            }
            {
                // typecast p[i], index as lowercase letter and encrypt with k
                c[i] = (((int)p[i] - 97) + k) % 26;
            }
        }
        else // = if not letters
        {
            printf("%c", p[i]);
        }
        // print the crypted message and exit
        printf("cyphertext: %s\n", c);
        return 0;
    }
}
else
{
    // print error
    printf("There's an error in the commande line argument\n");
    // return 1 (error)
    return 1;
}}`
2

Multiple problems.

One problem is your formulae, you correctly shift letters to range 0..25, add the key and apply the modulo operator. But you forget to shift the characters back into the letter range (+'A' or equivalently +65, and +'a' or +97 at the end of the lines).

The code

        {
            (islower(p[i]));
        }

does not make any sense to me and should probably be deleted. Might have been an attempt of if (islower(p[i])), but all relevant characters that are alphabetic are either upper- or lowercase, so a simple else works. You could use if (isupper(p[i])) { ... } else if (islower(p[i])) { ... } if you want to get rid of the isalpha. Result should be the same.

Third problem is that you have two variables, c and p that both point to the same memory, so they mean the same and can be used interchangeably. In C, strings are pointers to the first character of the string, with the end marked by a byte of value zero.

That last one is not a problem in itself (you process each letter exactly once, and don't need their original value again), but in this case you should not do anything on non-letter characters. Instead, you print those, leading to extra output.

An alternative is to print all characters immediately instead of writing them into memory first. In that case, you would have to print in all cases. But printing in one of three is always wrong.

2
  • Thanks a lot @Blauelf. I've changed my 'else' for an 'else if' and removed the 'isalpha'. Also, I realized why my loop wasn't incrementing (it changed only the first character of the string) and moved the 'printf("ciphertext: %s\n", c); return 0;' out of the loop, right after it. It now works and returned a 5/5! Now Vigenère! :)
    – Phil
    Mar 8 '18 at 14:03
  • Oops, totally missed that last one. Great it works now.
    – Blauelf
    Mar 8 '18 at 15:16

You must log in to answer this question.

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