vigenere file produces two different different outputs throug debug50 and ./vigenere

Question

This is for pset2 vigenere; Through check50, my code works, except for one case: "a does not produce a under key 'a'". If you copy my code, compile it, and run it, it works fine sometimes and other times ceases to be correct (I get 'a' plus some other non-alphabetic characters after it). I went through the debugger to find out whats wrong, but EVERYTIME I use the debug50, it produces what I want (which is 'a'). I don't if its something wrong with my code, or with debug50, or with multiple iterations of my code, vigenere file mutates after ever run? Copy and paste, compile, and run my code through debug50 and through ./vigenere, and see for yourself.

#include <stdio.h>
#include <cs50.h>
#include <string.h>
#include <ctype.h>
int main (int argc, string argv[])

{
string key =  argv[1];

// too many too little arguements
if (argc != 2)
{
    printf("Usage: %s k\n", argv[0]);
    return 1;
}

//check if its all char
int i = 0;
int length_key;
for(length_key = 0; key[length_key] != '\0'; ++length_key);
while (i < length_key)
{
    if (isalpha(key[i]))
    {
        key[i] = tolower(key[i]);
        i += 1;
    }
    else
    {
        printf("Usage: %s k\n", argv[0]);
        return 1;
    }
}

//certified key is ready to use

//encipher function
if (argc == 2)
{
    //get the plaintext
    string plaintext = get_string("plaintext: ");
    int length_pt;
    for(length_pt = 0; plaintext[length_pt] != '\0'; ++length_pt);
    int index_key = 0;
    int index_plaintext = 0;
    char output[length_pt+1];

    while (index_plaintext < length_pt)
    {
        char p_x = plaintext[index_plaintext];
        char k_x = key[index_key%(length_key)];

        //if alphabetic
        if (isalpha(p_x))
        {
            //not capital
            if islower(p_x)
            {
                int pt_ascii = p_x;
                int shift = k_x-97;
                int shifted = (pt_ascii + shift);
                if (shifted > 122)
                {
                    int shifted_new = shifted%122;
                    shifted = 96 + shifted_new;
                }
                char new_pt = shifted;
                output[index_plaintext] = new_pt;
                index_plaintext += 1;
                index_key += 1;
            }

            //capital
            if isupper(p_x)
            {
                int pt_ascii = p_x;
                int shift = k_x-97;
                int shifted = (pt_ascii + shift);
                if (shifted > 90)
                {
                    int shifted_new = shifted%90;
                    shifted = 64 + shifted_new;
                }
                char new_pt = shifted;
                output[index_plaintext] = new_pt;
                index_plaintext += 1;
                index_key += 1;
            }
        }

        //not an alphabetic char
        else
        {
            output[index_plaintext] = p_x;
            index_plaintext += 1;
        }

    }

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

}

}

I know its long, and far from elegant.

Answer 1

The code uses the technique of accumulating the encoded text in a buffer array, output, and then printing out the entire string at the end. The problem is that the end of string marker, \0 is never written to output. So, when there's random trash data in the buffer (remember, it isn't initialized to all binary 0's), printf will try to print everything starting at the address of output until it encounters \0, which is represented by a byte of 0x00. You should be easily able to fix this.

And now, some programming tips and best practices. ;-)

While the code is "correct" (and demonstrates skill at finding solutions), you should get familiar with functions as you go. Most notably, you could replace for loops in your code with strlen() calls - something you will use constantly. At this point, you should ask yourself "Does what I'm trying to do seem like something really common? Then there's probably a function for it somewhere." Go look for it. Google "How to...in c." You'll learn so much along the way!

It's common and good practice to declare and initialize in the same line too. It helps to reduce the number of lines and makes their usage immediately clear. For instance,

int length_pt = strlen(plaintext);

Also, i++; is perhaps the most commonly written line of code by programmers everywhere! ;-)

Finally, a note on unnecessary code. The statemet if (argc == 2) and the curly braces that encapsulate the rest of the program are unnecessary. Earlier, if (argc != 2) verifies that it is 2. If not, the program terminates, so this becomes totally unnecessary. Having unnecessary code can lead to errors, both when originally written or days to years later when the code has to be updated. If the (new or same) person modifying the code isn't sure what's going on, that extra code could be misinterpreted, ignored, or just plain missed. It's always better not to have any extra code in a program.

If this answers your question, please click on the check mark to accept. Let's keep up on forum mainteance. ;-)