I've been busting my head over this all day. So the main problem (I think) is that I'm having difficulties synchronizing the chars from the key with the chars from the sentence after a non-alpha character is on the way. The problem is that int "i" (in the code in this post) keeps going up even when there are non-alpha characters, so in my equation later, the key also moves one position away from the letter that should be used. That's why I created "o" which should increase every time the programme gets to a non-alphabetic character. Then its value gets subtracted and that way cuts the gap between the two strings (key and sentence). That was a brief explanation on the most important point in my code from where I think the problem comes. I hope that any of you guys would give me a tip on how to fix my loop, offer other non-loop solutions or finds a different problem in the code.

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

int main(int argc, string argv[])
    while(argc != 2)                    //yells at the user if more or less than 2 arguments
        printf("Enter a key.\n");
        return 1;

    string s = GetString();         //user gives input

    for(int i = 0, n = strlen(s); i < n; i++)       //goes through every character of that input
        for(int o = 0; isalpha(s[i]); o++)              //loop used to cope with dishronyzation because of non-alpha chars
            if(isalpha(s[i]))               //checks if its a letter character
                o = o - 1;                  //getting rid of "o" value because the char is a letter
                int d = strlen(argv[1]);

                if(isupper(s[i]))               //checks if its an uppercase letter
                    int f = (i - o) % d;        //synchronizes every character from the key with every character from the sentence
                    int h = argv[1][f] - 65;    //because of "o", excluding the difference that non-alpha chars give 
                    char v = s[i] - 65;
                    char a = (v + h) % 26;               //uses modulo, so that the output is only alphabetical
                    char l = a + 65;
                    printf("%c", l);                //prints the encrypted character                    

                if(islower(s[i]))               //does the same process for lowercase letters
                    int w = (i - o) % d;
                    int e = argv[1][w] - 97;
                    char t = s[i] - 97;
                    char y = (t + e) % 26;
                    char u = y + 97;
                    printf("%c", u);
                printf("%c", s[i]);                 //prints non-alpha chars
  • You are pointing in the right direction when you say you need something other than i to keep track of the progress of your key. That being said, are you sure you want to enter a second loop, what purpose does this serve you?
    – dLopez
    Aug 6, 2016 at 18:37
  • Well, it gives me the opportunity do declare "o" just once. Otherwise if I declare "o" into the first loop, its value would be 0 because it would be declared every single time that the loop goes through a different s[i] character. Aug 6, 2016 at 18:40
  • I don't think you completely understand how nested for loops work. Basically, on each iteration of your outer loop, you are going through the entirety of your inner loop. I don't believe this is what you want. Also the inner for statement should take your program on an infinite loop since there is no condition to be met for the loop to be exited.
    – dLopez
    Aug 6, 2016 at 18:45
  • Yes, I just figured that out and changed it to combine the two loops like this: for(int i = 0, n = strlen(s), o = 0; i < n; i++, o++) But still it doesn't seem to work fine. Aug 6, 2016 at 18:50
  • That seem right. Did that fix your program? Is it now passing all of check50 tests?
    – dLopez
    Aug 6, 2016 at 18:53

1 Answer 1


There are a couple of issues to be dealt with here. First, using two loops is a bad approach. You want to use a loop for something that must be done every time, not something that may or may not be done. In this case, while every letter is examined in the plain text, not every key letter is consumed in the process.

Next, tracking the key with the same index, i, as used to track the key is a problem for the same reason. Every char in the plaintext is processed, whether encoded or just echoed, while key chars are only consumed when a letter is encoded.

There's an old saying in programming, "KISS - Keep It Simple, Stupid!" - something I tell myself at some point during every project. In this case, the simple thing to do is to track the key index with a simple int variable. When a letter is encoded, just increment the index var. Also, be sure to declare and initialize this var before and outside of the for loop.

Finally, there's another problem lying in wait that you haven't discovered. The code does not handle the condition where the plaintext and the key are not the same case - one is upper case and the other lower case. I'll leave it to you to write this code as you probably haven't thought about it.

This should get you going, or at least give you plenty to think about.

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

  • I solved both the problems, I just need to handle the last error now. Aug 7, 2016 at 11:19

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .