0

I have been working on this Vigenere cipher for about 8 hours straight. Can you please help me? I think that main problem is around the algorithm - I don't know how to utilize the keylength (I know I need to get the mod of it somehow).

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

int main(int argc, char *argv[])
{
    if(argc != 2)
    {
        printf("Bad!");
        return 1;
    }
    int keylen = strlen(argv[1]);
    char *key = argv[1];
    for (int i = 0; i < keylen; i++)
        {
            if(isupper(key[i]))
            {
                key[i] = key[i]-65;
            }
            else if(islower(key[i]))
            {
                key[i] = key[i]-97;
            }
        }
    printf("Plaintext: ");
    string p = GetString();
    int k = 0;
    for (int i = 0; i < strlen(p); i++)
    {
        if(isalpha(p[i]))
        {
            if(isupper(p[i]))
            {
                p[i] = ((p[i]-65)+(key[k % keylen]))%26 + 65;
            }
            else if(islower(p[i]))
            {
                p[i] = ((p[i]-97)+(key[k % keylen]))%26 + 97;
            }
            k++;
        }
        else
        {
            //I need to skip over antyhing that isn't a letter
            p[i] = p[i];
        }
    }
    printf("Ciphertext: %s\n", p);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
user3342048
  • 55
  • 1
  • 10
  • Ok, you changed the code in the question. So are you still having a problem, and if so, what is the problem? – user3386109 Apr 22 '14 at 01:39
  • Yeah, still having a problem. So I changed it to "p[i] = ((p[i]-65)+(key[i % keylen]));" And it still isn't working... – user3342048 Apr 22 '14 at 01:45
  • I think I see the problem, updated my answer – user3386109 Apr 22 '14 at 01:46
  • Thanks so, so much for helping me. So I did that -- added mod 26 giving me "p[i] = ((p[i]-65)+(key[i % keylen])%26);" (and a few other variations). Still no dice...Okay so I am getting ca�go� instead of caqgon...something here....is working, but something here isn't. I tried adding %26 in a few places, but no dice. The keyword was "baz" so I feel like it has something to do with the later letters. I feel like they aren't wrapping around the alphabet. Wouldn't mod 26 be the way to solve this? – user3342048 Apr 22 '14 at 02:03
  • Q1: what was the original text? Q2: this is the encryption side right? – user3386109 Apr 22 '14 at 02:10
  • The original text was "barfoo" with the keyword "baz", and the output should be "caqgon" but it is "ca�go�". (Using: p[i] = (p[i]-65)+(key[i % keylen]);) So something is working, but the later letter - z being hte shift - isn't working for some reason. Mod 26 isnt' solving it. And thank you so, so much for helping me - again. – user3342048 Apr 22 '14 at 04:50
  • Yes, the `r` shifted by `z` is a problem since the sum will be outside the range of 0 to 25. In a previous comment you mentioned that you tried `p[i] = ((p[i]-65)+(key[i % keylen])%26);` I see two problems with that. The parentheses are in the wrong place, the modulo will be applied to the `key`, but the modulo needs to be applied to the sum `((p[i]-65)+key[i % keylen])`. Also, that line leaves out the last step where the 65 is added back in. Note that it sometimes helps, with complicated expressions, to define a couple of intermediate variables and do the calculation on multiple lines. – user3386109 Apr 22 '14 at 05:11
  • When I try: p[i] = (p[i]-65)+(key[i % keylen])%26 + 65; I get: vt�z��, which is completely wrong. Using only p[i] = (p[i]-65)+(key[i % keylen]) gives me a more correct-looking answer, even if it is still not working. Am I implementing it incorrectly? And thanks so, so much. UPDATE: So I tried : p[i] = ((p[i]-65)+(key[i % keylen]))%26 + 65; And now I'm getting : vtjzhg which is wrong, but at least the z is shifting...why is it not shifting to the correct value? – user3342048 Apr 22 '14 at 05:20
  • Modulo has priority over addition, so the code is doing `x + (y%26)` where it needs to be doing `(x+y) % 26`. You are one set of parentheses short of the goal :) – user3386109 Apr 22 '14 at 05:25
  • Oh wow! I just noticed the `int i = 0; i > keylen; i++`. The first `for` loop isn't working at all. – user3386109 Apr 22 '14 at 05:28
  • Oh really? What is wrong with it? Thanks so much for the observation! Really, I've been working on this thing for way too long, and I appreciate your help so, so much. – user3342048 Apr 22 '14 at 05:35
  • I've updated the answer with an explanation. – user3386109 Apr 22 '14 at 05:36
  • Okay, thanks so, so much for helping me. I am very close, I think. I've updated my code -- I think I am really really close, but commas/spaces/non alphabetic characters mess it up. Can you take a look? Again, thanks so, so much. – user3342048 Apr 22 '14 at 06:04
  • Hmm, the variable `i` increments each time through the loop, regardless of whether the input is an alpha character, or not. But that means that the index into the key array also increments each time through the loop. Since you're trying to skip non-alpha characters, you might need to add a variable `k` to keep track of where you are in the key, and only increment `k` when the input character is an alpha character. – user3386109 Apr 22 '14 at 06:13
  • Yeah, that's what I gathered...the i incrementing being the problem. So this variable - I thought about that, but was not sure how to use it. Would k replace i within the part of the program that includes "p[i] = ((p[i]-65)+(key[i % keylen]))%26 + 65;"? And again,t hanks so, so much. – user3342048 Apr 22 '14 at 06:18
  • Okay, I've updated it...for me it works, but for some reason my checker says that it is not working. Is there anything that could be wrong with it? And yet, yes, again, thank you. – user3342048 Apr 22 '14 at 06:26
  • Am I correct in assuming that the checker is a decryption program that someone else wrote? If so, do you have any samples of plain text, key, cipher text that make the checker happy? – user3386109 Apr 22 '14 at 06:31
  • That's the thing - this is for a CS class. The checker is an autograder. I have a lot of examples from the checker, but when I input them, my program passes. Yet for some reason, my program is failing...I'm so sorry to keep bothering you, but do you have any idea? Again, thanks so, so much for all of this. This program has been rdiving me insane today haha – user3342048 Apr 22 '14 at 06:38
  • Here's what's going on...[link](http://imgur.com/IzfRBwq) – user3342048 Apr 22 '14 at 06:42
  • The first error seems to say that `a` shouldn't be encrypted as `a` when using a key `a`. Don't know why that would be a problem. I wonder if maybe the problem is the `\n` in the last printf. – user3386109 Apr 22 '14 at 06:55
  • Yeah, the weird thing is that I get a when I input a with the key a. I tried taking out the /n in the last printf, and that didn't change anything. I don't think that's the problem, but I don't get why I am getting an error. One thing that is a problem is that it's not rejecting keywords with numbers and non-alphabetic characters inside them. How might I do this? I tried doing an if statement over most of the program - tagging onto where if argc is not equal to 2, and that didn't work. Also, what is "expected exit code of 1"? – user3342048 Apr 22 '14 at 13:44
  • Ah, I got it!!! Just wanted to thank you again for all the help. This program was seriously driving me mad. No one in my group home knows anything about computers or code (or math, for that matter), and I was really on my own (with Google, of course). You helped me tremendously - thanks! – user3342048 Apr 22 '14 at 14:33
  • @user3386109 - No, they should not delete this question. We're trying to build a repository of good questions and answers, not police academic policies of other places. If it's a good question with a good answer, it belongs here. – Brad Larson Apr 25 '14 at 20:04

1 Answers1

3

The first for loop has a problem. The condition is checking for i > keylen when it should be checking for i < keylen.

Also when computing the next output value, the steps should be

(p[i]-'A') results in a number between 0 and 25
adding (key[i % keylen]) results in a number between 0 and 50
apply modulo 26 so the number is between 0 and 25 (this is the missing step)
then add 'A' to get the output

Note: Avoid using hard-coded numbers for character constants. For example, use 'A' instead of 65, and 'a' instead of 97.

user3386109
  • 34,287
  • 7
  • 49
  • 68
  • 3
    Note that rather than using 65, you should use the constant `'A'`; it makes it clearer what you're dealing with — and makes it easier to see how the lower-case equivalent code matches the upper-case code. – Jonathan Leffler Jul 30 '16 at 14:35