CS50, substitution, check50 fails in duplicate tests with time out

Question

My problem might be related to cs50x 2020 - pset2 - substitution - duplicate characters in key or CS50 - pset2 - substitution but I still can't figure it out.

When I run the test manually it passes it, but when I run check50 it fails with:

:( handles duplicate characters in key
    timed out while waiting for program to exit
:( handles multiple duplicate characters in key
    timed out while waiting for program to exit

Where do you think the check50 fails its last two test

Edit: I remove my code because I just found out about the "Academic Honesty" clauses of the course and this question /answer thread may violate them.

The error message is clear: The program was tested with a key that does not contain a permutation of the 26 Latin characters. So in addition to testing whether the key consists of only letters, you must test whether a letter has already been used in the key. Also, be consistent with the naming. Don't call the key ciphertext and after you've set `ciphertext = argv[1]`, don't mix both names. — M Oehm, Jun 01 '21 at 17:44
@konstantinos. No - I was wrong :-) the strlen is in the init part so it is not as bad as I made out. I'm just so used to seeing `for(int i = 0; i < strlen(str); i++)` I didn;'t read it carefully enough. Now you've gone too far. `plaintext` might not be 26. — John3136, Jun 01 '21 at 17:45
@John3136 No, you have a point. When I was writing it I thought perhaps I shouldn't harcode 26 so I can copy/paste this part of my code to other little projects might it be the case. But since I'm an optimization freak, I should have it removed it. Or more ideally just assign the number to a variable in the beginning of the main. — Konstantinos, Jun 01 '21 at 17:47
`lowcase_ciphertext[plaintext[i] - 'A'] + 'A' - 'a'` seems odd since `'A' - 'a' = -32`. — 001, Jun 01 '21 at 17:48
If you don't want to hardcode then just call strlen() once and store the result. Remember that plaintext may not be 26 chars (could be more or less) — John3136, Jun 01 '21 at 17:49

score -1 · Accepted Answer · edited Jun 06 '21 at 00:05

Never giving up, I found the solution. The problem was that I didn't understand some details (immutability) of how string arrays work so I did some lame workarounds (remember the "aaaaaaaa" assignment...). Eventually Initializing an array of characters with a string variable, How to convert a string to character array in c (or) how to extract a single char form string?, C char array initialization plus implementing the rule that the key should contain each chracter exactly once (which I do not find necessary!) led me to the solution which passes all the tests. I post it here:

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

int main(int argc, string argv[])
{
    // Remember argc before argv control; otherwise segmentation fault
    if (argc < 2)
    {
        printf("Usage: ./substitution key.\n");
        return 1;
    }

    if (strlen(argv[1]) != 26)
    {
        printf("Key must contain 26 characters.\n");
        return 1;
    }

    string key = argv[1];
    int key_length = strlen(key);

    for (int i = 0; i < key_length; i++)
    {
        if (!isalpha(argv[1][i]))
        {
            printf("Key must contain 26 characters.\n");
            return 1;
        }
    }

    // Error when key contains duplicate characters
    for (int i = 0; i < key_length; i++)
    {
        for (int j = 0; j < key_length; j++)
        {
            if (i == j)
            {
                continue;
            }
            if (islower(key[i]))
            {
                char current_key_letter = key[i];
            }
            else
            {
                char current_key_letter = key[i] + 'A' - 'a';
            }
            if (key[i] == key[j])
            {
                printf("Key must contain 26 unique characters.\n");
                return 1;
            }
        }

    }

    string plaintext = get_string("plaintext: ");

    int plaintext_length = strlen(plaintext);
    char output_text[plaintext_length];
    strcpy(output_text, plaintext);

    char lowcase_key[key_length];
    strcpy(lowcase_key, key);

    for (int i = 0, n = strlen(lowcase_key); i < n; i++)
    {
        lowcase_key[i] = tolower(key[i]);
    }

    for (int i = 0, n = plaintext_length; i < n; i++)
    {
        if (islower(plaintext[i]))
        {
            output_text[i] = lowcase_key[plaintext[i] - 'a'];
        }
        if (!isalpha(plaintext[i]))
        {
            output_text[i] = plaintext[i];
        }
        if (isupper(plaintext[i]))
        {
            output_text[i] = lowcase_key[plaintext[i] - 'A'] + 'A' - 'a';
        }
    }
    printf("ciphertext: %s", output_text);
    printf("\n");
    return 0;
}

There are more elegant solutions, although I really like the - 'A'] + 'A' - 'a' part, but that's OK for today. Thanks all for comments!

CS50, substitution, check50 fails in duplicate tests with time out

1 Answers1