pset2 substitution not working

Question

im kinda stuck with pset2, substitution. Any help is appreciated. When trying to convert the plaintext into the ciphertext, the formula just doesnt do anything. I suppose there is something wrong with it, but what is?

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

bool letters(string x, int y);
string cipher(string x, string g, int y);

int main(int argc, string argv[])
{
    int y = strlen(argv[1]);
    bool check = letters(argv[1], y);
    if (argc != 2 || y != 26 || check != true)
    {
        printf("Usage ./substitution key\n");
        return 1;
    }
    string plaintext = get_string("plaintext: ");
    string ciphertext = cipher(plaintext, argv[1], y);
    printf("ciphertext: %s\n", ciphertext);
}

bool letters(string x, int y)
{
    bool j;
    for (int i = 0; i < y; i++)
    {
        for (int k = 1 + i; k < y; k++)
        {
            if (toupper(x[i]) == toupper(x[k]))
            {
                j = false;
                return j;
            }
        }
        if (!isalpha(x[i]))
        {
            j = false;
        }
        else
        {
            j = true;
        }
    }
    return j;
}

// itinerates over the plain text 
string cipher(string x, string g, int y)
{
    for (int i = 0; i < y; i++)
    {  
        // itinerates over each letter of the plaintext and checks if it matches the key letters. if it 
          //does, replaces the ith letter for the gth one
        for (int k = 0, len = strlen(g); k < len; k++)
        {
            int h = 0;
            h = (g[k] % 90) + 65;
            h += k;
            if (isalpha(x[i]))
            {
                if (isupper(g[k]) && toupper(x[i]) == h)
                {
                    x[i] = g[k];
                }
            }
        }
    }
    return x;
}

The cipher function isnt finished yet, but shouldnt it convert the plaintext if it is lowercase and the key is uppercase?

The section with comments on the code is the one that isnt working.

Answer 1

I don't think h = (g[k] % 90) + 65; is doing what you want it to do.

If your cipher is ABCDEFG.... (just the actual alphabet), for the first time you go through that loop (i = 0, k = 0), h = (g[k] % 90) + 65; -> h = ('A' % 90) + 65; -> h = (65) + 65; -> 130.

h += k; -> 130.

Then, toupper(x[i]) == h will never be true, because 130 isn't the ASCII value for any capital letters.

It looks like you're trying to go through every letter of the plain text, then check that letter against every letter the cipher to see if it works. You shouldn't have to loop over the cipher for every letter. If the first letter of the plain text is "e", you should go directly to the 5th (starting from 1) letter of the cipher, and swap "e" for that letter.

Also, it was really hard to figure out what your code was doing, especially because in string cipher(string x, string g, int y) it's hard to remember what string x, string g, int y mean. I had to keep going back to the main method to remember what each one meant. Especially if other people are going to read your code, it's helpful to use variable names that mean something. Maybe string plaintext, string cipher, int length_of_cipher.

I hope this helps!