Troubles with CS50's pset2 "substitution" output

Question

I can't seem to understand what is the problem with my program. The assignment is CS50's "substitution" from problem set of Week 2. The goal is to create a program which, after the user typed a 26-chars key as argv[2], converts a text the user types in to an encrypted text in which the regular alphabet is switched with the key. My code works almost completely fine, but with one or two letters (which change every time) for each text i want to encrypt, the output is wrong.

e.g. ./substitution YUKFRNLBAVMWZteogxhcipjsqd

plaintext: This is CS50

cyphertext: Cbah ah DH50

(in which the letter C should be K, not D)

or even:

./substitution NQXPOMAFTRHLZGECYJIUWSKDVB

plaintext: This is CS50

cyphertext: Kfki ki DI50

(in which, for a reason to me unknown the letter T and the letters i are both 'k' and both are wrong)

I've been desperately looking for the cause of this bug for hours, but I can't seem to find it. I'd really appreciate if one of you could help me find it. Thanks in advance.

(If you need further clarification on the output, here's the 'Check50' tool's message:

:) substitution.c exists

:) substitution.c compiles

:( encrypts "A" as "Z" using ZYXWVUTSRQPONMLKJIHGFEDCBA as key expected "ciphertext: Z...", not "cyphertext: A..."

:( encrypts "a" as "z" using ZYXWVUTSRQPONMLKJIHGFEDCBA as key expected "ciphertext: z...", not "cyphertext: a..."

:( encrypts "ABC" as "NJQ" using NJQSUYBRXMOPFTHZVAWCGILKED as key expected "ciphertext: NJ...", not "cyphertext: CF..."

:( encrypts "XyZ" as "KeD" using NJQSUYBRXMOPFTHZVAWCGILKED as key expected "ciphertext: Ke...", not "cyphertext: Ke..."

:( encrypts "This is CS50" as "Cbah ah KH50" using YUKFRNLBAVMWZTEOGXHCIPJSQD as key expected "ciphertext: Cb...", not "cyphertext: Cb..."

:( encrypts "This is CS50" as "Cbah ah KH50" using yukfrnlbavmwzteogxhcipjsqd as key expected "ciphertext: Cb...", not "cyphertext: Cb..."

:( encrypts "This is CS50" as "Cbah ah KH50" using YUKFRNLBAVMWZteogxhcipjsqd as key expected "ciphertext: Cb...", not "cyphertext: Cb..."

:( encrypts all alphabetic characters using DWUSXNPQKEGCZFJBTLYROHIAVM as key expected "ciphertext: Rq...", not "cyphertext: Rr..."

:) handles lack of key

:) handles invalid key length

:) handles invalid characters in key

:) handles duplicate characters in key

:) handles multiple duplicate characters in key)

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

// create the stirng variable alphabet for the key substitution
string alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

int main(int argc, string argv[])
{
    // get the key for the alphabet substitution as the second argument
    string key = argv[1];
    // if the user enters no key, instructions are given
    if (argc == 1)
    {
        printf("Usage: ./substitution key\n");
        return 1;
    }
    // loop to go through all the key chars and check them
    for (int i = 0; i < 26; i++)
    {
        // if at least one of the chars is not a letter, print an error and return 1
        if (!isalpha(key[i]))
        {
            printf("Key can contain only alphabetic characters.\n");
            return 1;
            break;
        }
        // adds another loop to confront a char with all the other 25 to see if there aare duplicates
        for (int j = i + 1; j < 26; j++)
        {
            // if there is a repeated char, print an error and return 1
            if (key[i] == key[j] || key[i] == ' ')
            {
                printf("Key doesn't allow repeating characters.\n");
                return 1;
                break;
            }
        }
    }
    // print an error and return 1 from main if the key is not 26 chats
    if (strlen(key) != 26)
    {
        printf("Key must contain 26 characters.\n");
        return 1;
    }
    else
    {
        // print 'plaintext: ', prompting the user for the string he wants to encrypt, using get_string
        string pltxt = get_string("plaintext: ");
        string cytxt = pltxt;
        // get the lenght of the plaintext variable
        int lentxt = strlen(pltxt);
        // loop that goes through all the chars of the plaintext
        for (int n = 0; n < lentxt; n++ )
        {
            // check if the char is a letter
            if (isalpha(pltxt[n]))
            {
                // loop that goes through all the alphabet letters to see which one the current letter is and to change it with the keys one
                for (int m = 0; m < 26; m++)
                {
                    if (toupper(pltxt[n]) == alphabet[m])
                    {
                        // conditions that keep the cases of the substituted chars
                        if (islower(pltxt[n]))
                        {
                            pltxt[n] = tolower(key[m]);
                        }
                        else if (isupper(pltxt[n]))
                        {
                            pltxt[n] = toupper(key[m]);
                        }
                    }
                }
            }
            // if char is not letter, let it as it is
            else
            {
                cytxt[n] = pltxt[n];
            }
        }
        // output 'cyphertext: ', followed by the encrypted string and a newline
        printf("cyphertext: %s\n", cytxt);
        // exit the program by returning 0 from main
        return 0;
    }
}

Answer 1

In the loop

                for (int m = 0; m < 26; m++)
                {
                    if (toupper(pltxt[n]) == alphabet[m])
                    {
                        // conditions that keep the cases of the substituted chars
                        if (islower(pltxt[n]))
                        {
                            pltxt[n] = tolower(key[m]);
                        }
                        else if (isupper(pltxt[n]))
                        {
                            pltxt[n] = toupper(key[m]);
                        }
                    }
                }

Multiple substitutions may occur.

For example, in the first example

./substitution YUKFRNLBAVMWZteogxhcipjsqd

plaintext: This is CS50

1. C is substituted to K
2. K is substituted to M
3. M is substituted to Z
4. Z is substituted to D

And as a result C is substituted to D.

To prevent this, you should break from the loop after the first hit and let substitution happen only at most once for each characters.

                for (int m = 0; m < 26; m++)
                {
                    if (toupper(pltxt[n]) == alphabet[m])
                    {
                        // conditions that keep the cases of the substituted chars
                        if (islower(pltxt[n]))
                        {
                            pltxt[n] = tolower(key[m]);
                        }
                        else if (isupper(pltxt[n]))
                        {
                            pltxt[n] = toupper(key[m]);
                        }
                        break; // add this
                    }
                }