Search code examples
ccs50vigenere

CS50 Vigenere - Output Incorrect


Man I thought I had it! I've been working on the Vigenere problem and have gotten close but keep getting this error when I check. It looks like there's problem when the key has to loop back around. Thoughts?

Here is the error:

:) vigenere.c exists

:) vigenere.c compiles

:) encrypts "a" as "a" using "a" as keyword :( encrypts "world, say hello!" as "xoqmd, rby gflkp!" using "baz" as keyword \ expected output, but not "xoqmj, yfz gflkp!\n"

:( encrypts "BaRFoo" as "CaQGon" using "BaZ" as keyword \ expected output, but not "CaQAun\n"

:( encrypts "BARFOO" as "CAQGON" using "BAZ" as keyword \ expected output, but not "CAQAON\n"

:) handles lack of argv[1]

:) handles argc > 2

:) rejects "Hax0r2" as keyword

and here's my code:

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

#define FALSE 0
#define TRUE 1

int main(int argc, string argv[])
{
    string key = argv[1];

    if (argc!= 2)
    {
        printf("please provide only one perameter \n");

        // stop the program
        return 1;
    }

    // iterate over the key to make sure its all alpha  
    int i,j;
    for (i = 0, j = strlen(key); i < j; i++)
    {
        if (!isalpha(key[i]))
        {
            printf("please use only alphanumeric values \n");
            return 1;
        }
    }

    // now we have a key, "key" from the ONE perameter that is all alpha

    string message = GetString();
    int k = 0;
    int keyindex;

    for (i = 0, j = strlen(message); i < j; i++, k++)
    {
        if (isalpha(message[i]))
        {
            keyindex = k % strlen(argv[1]);
            // covering the upper case letters
            if (isupper(message[i]))
            {
                // covering the upper case letters with upper case key letters

                if isupper(key[i])
                {
                    // print cipher according to two upper case
                    int cipher = ((message[i] - 65 + key[keyindex] - 65) % 26)
                        + 65;
                    printf("%c", cipher);
                }
                else
                {
                    // print according to upper case message lower case key
                    int cipher = ((message[i] - 65 + key[keyindex] - 97) % 26)
                        + 65;
                    printf("%c", cipher);
                }
            }
            // this is for the non upper case letters
            if (islower(message[i]))
            {
                if isupper(key[i])
                {
                    // print cipher according to lower case message and
                    // upper case key letter
                    int cipher = ((message[i] - 97 + key[keyindex] - 65) % 26)
                        + 97;
                    printf("%c", cipher);
                }
                else
                {
                    // print according to lower case message  and lower case key
                    int cipher = ((message[i] - 97 + key[keyindex] - 97) % 26)
                        + 97;
                    printf("%c", cipher);
                }
            }

        }
        // non alpha symbols
        else
        {
            printf("%c", message[i]);
        }
    }

    // end program after iterating
    printf("\n");

}

Solution

  • Problems with your program:

    1) Sytax error that should keep it from compiling:

    if isupper(key[i]) -> if (isupper(key[i]))

    There are two of these so make sure to fix them both.

    2) Incrementing k

    int k = 0;
    ...
    for (i = 0, j = strlen(message); i < j; i++, k++)
    {
        if (isalpha(message[i]))
        {
            keyindex = k % strlen(argv[1]);
    

    There two was to approach this, either increment k on ever character or increment k on every letter. The designer of this problem chose letter so instead we need to do:

    int k = 0;
    ...
    for (i = 0, j = strlen(message); i < j; i++)
    {
        if (isalpha(message[i]))
        {
            keyindex = k++ % strlen(argv[1]);
    

    3) Use keyindex now that we've defined it:

    if isupper(key[i]) -> if isupper(key[keyindex])

    There are two of these so make sure to fix them both.

    Applying these changes and a little bit of style cleanup, we get:

    #include <stdio.h>
    #include <cs50.h>
    #include <stdlib.h>
    #include <ctype.h>
    #include <string.h>
    
    int main(int argc, string argv[])
    {
        if (argc != 2)
        {
            printf("please provide only one parameter \n");
    
            return 1; // stop the program
        }
    
        string key = argv[1];
    
        // iterate over the key to make sure it's all alpha  
    
        for (int i = 0, j = strlen(key); i < j; i++)
        {
            if (!isalpha(key[i]))
            {
                printf("please use only alphanumeric values \n");
    
                return 1;
            }
        }
    
        // now we have a key, "key" from the ONE parameter that is all alpha
    
        string message = GetString();
    
        for (int i = 0, k = 0, j = strlen(message); i < j; i++)
        {
            if (isalpha(message[i]))
            {
                int keyindex = k++ % strlen(key);
    
                if (isupper(message[i])) // covering the upper case letters
                {
                    if (isupper(key[keyindex]))
                    {
                        // print cipher according to both upper case
                        int cipher = ((message[i] - 'A' + key[keyindex] - 'A') % 26) + 'A';
                        printf("%c", cipher);
                    }
                    else
                    {
                        // print cipher according to upper case message and lower case key
                        int cipher = ((message[i] - 'A' + key[keyindex] - 'a') % 26) + 'A';
                        printf("%c", cipher);
                    }
                }
                else // this is for the non upper case letters
                {
                    if (isupper(key[keyindex]))
                    {
                        // print cipher according to lower case message and upper case key letter
                        int cipher = ((message[i] - 'a' + key[keyindex] - 'A') % 26) + 'a';
                        printf("%c", cipher);
                    }
                    else
                    {
                        // print cipher according to both lower case
                        int cipher = ((message[i] - 'a' + key[keyindex] - 'a') % 26)  + 'a';
                        printf("%c", cipher);
                    }
                }
    
            }
            else // non alpha symbols
            {
                printf("%c", message[i]);
            }
        }
    
        printf("\n"); // end program after iterating
    
    }
    

    Your code duplicates a lot of logic that you could combine with minor changes -- duplicated logic is one way that hard to find errors creep into code.