CS50 - C -problemset 2 - substitution - output not valid ASCII text

Question

Thats my first post on stackoverflow so i hope i'm doing this properly, if not let me know.

i have a problem with the code, but i cant see were. The CS50 check gives me un output error: ** ":( encrypts "This is CS50" as "Cbah ah KH50" using yukfrnlbavmwzteogxhcipjsqd as key. Cause: output not valid ASCII text"**

Thats the code:

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



int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./substitution\n");
        return 1;
    }

    string key = argv[1];

    if (strlen(key) != 26)
    {
        printf("The key must contain 26 characters\n");
        return 1;
    }

    for(int i = 0; i < strlen(key); i++)
    {

    if (!isalpha(key[i]))
    {
        printf("Usage: ./substitution\n");
        return 1;
    }

    }

    for (int i = 0; i < strlen(key); i++)
    {
        for(int j = i + 1; j < strlen(key); j++)
        {
            if (toupper(key[i]) == toupper(key[j]))
            {
                printf("Usage: substitution key\n");
                return 1;
            }
        }
    }

    string plaintext = get_string("Plaintext: ");

    printf("ciphertext: ");

    for(int i = 0; i < strlen(plaintext); i++)
    {
        if(isupper(plaintext[i]))
        {
            int letter = plaintext[i] - 'A';
            printf("%c", key[letter]);
        }
        else if(islower(plaintext[i]))
        {
            int letter = plaintext[i] - 'a';
            printf("%c", tolower(key[letter]));
        }
        else if (plaintext[i] == ' ') // Handle spaces
        {
        printf(" ");
        }
        else
        {
            printf("%c",plaintext[i]);
        }
    }
    printf("\n");
    return 0;
}

Answer 1

I am responding to your request based upon my original comment above.

First off, even though I have analyzed other "CS50" issues in the past, I don't actually have the Harvard course installed on my system; however, that has not deterred me from analyzing and providing suggestions in the past. Having set that as my background, from what I have seen and studied of the course, the checker program can be fairly fussy with its analysis of code and answers, and what appears to have happened to you here is that the CS50 check process was expecting a string to be returned. However, you technically are printing out one character at a time. Though this might be pedantic, your code does not print out a character array ending with a terminator character ('\0').

So what I was suggesting in my comment would be to refactor the your cipher algorithm.

Following is the code block I'm focusing on.

    string plaintext = get_string("Plaintext: ");

    printf("ciphertext: ");

    for(int i = 0; i < strlen(plaintext); i++)
    {
        if(isupper(plaintext[i]))
        {
            int letter = plaintext[i] - 'A';
            printf("%c", key[letter]);
        }
        else if(islower(plaintext[i]))
        {
            int letter = plaintext[i] - 'a';
            printf("%c", tolower(key[letter]));
        }
        else if (plaintext[i] == ' ') // Handle spaces
        {
        printf(" ");
        }
        else
        {
            printf("%c",plaintext[i]);
        }
    }
    printf("\n");
    return 0;

Treating the encrypted/ciphered text as a string, following is a refactored version of that code block.

    string plaintext = get_string("Plaintext: ");

    char ciphertext[strlen(plaintext) + 1];         /* Define a character array large enough to hold the inputted string    */

    strcpy(ciphertext, plaintext);                  /* Use standard string function to copy entered string to work string   */

    for(int i = 0; i < strlen(ciphertext); i++)     /* Simplified for loop  */
    {
        if(isupper(ciphertext[i]))
            ciphertext[i] = toupper(key[ciphertext[i] - 'A']);  /* Just replace alphabetical characters with appropriate key value  */
        else if(islower(plaintext[i]))
            ciphertext[i] = tolower(key[ciphertext[i] - 'a']);  /* Just replace alphabetical characters with appropriate key value  */
    }

    printf("Ciphertext: %s\n", ciphertext);

    return 0;

The key points are as such:

• A work variable is defined that is large enough to contained the entered string.
• Using the standard "strcpy" function, the entered string is copied to this work string.
• Using a for loop, each character in the work string is evaluated if it is an uppercase or lowercase character, and if so, the encryption process is applied to that character.
• Once all character positions have been analyzed and processed, the work string is printed out.

Following was my test utilizing the noted string and key.

craig@Vera:~/C_Programs/Console/CSsub/bin/Release$ ./substitution yukfrnlbavmwzteogxhcipjsqd
Plaintext: This is CS50
Ciphertext: Cbah ah KH50

As I noted, I don't actually have the CS50 tools on my system, but you might try that as it appears that the checker process expects a string and not a consecutive printing of individual characters.