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

int shift(char c);

int main(int argc, string argv[])
    // Check that program was run with one command-line argument
    if (argc != 2)
        printf("Usage: ./vigenere key\n");
        return 1;
    else if (argc == 2)
        // Iterate over the provided argument to make sure all characters are alphabetic
        for (int i = 0; i < strlen(argv[1]); i++)
            if ((isdigit(argv[1][i]) != 0) || (!isalpha(argv[1][i])))
                printf("invalid keyword\n");
                return 1;

        // prompt user for input
        string plaintext = get_string("plaintext:  ");
        // initiate ciphertext array of same length as plaintext
        char ciphertext[strlen(plaintext)];
        // initiate cipherKey array 
        char cipherKey[strlen(argv[1])];
        // convert all argv[1] to int key, append them to an array
        for (int w = 0; w < strlen(argv[1]); w++)
            // fill cipherKey array of shifted values of agrv[1]
            cipherKey[w] = shift(argv[1][w]);
        printf("ciphertext: ");
        // if key is just one
        int textLength = strlen(plaintext);
        int keyLength = sizeof(cipherKey);

        for (int i = 0, j = 0; i < textLength;)
            // if char is upper case
            if (plaintext[i] >= 65 && plaintext[i] <= 90)
                ciphertext[i] = (char)((((plaintext[i] - 'A') + (int)cipherKey[j]) % 26) + 'A');
            // if char is lower case
            else if (plaintext[i] >= 97 && plaintext[i] <= 122)
                ciphertext[i] = (char)((((plaintext[i] - 'a') + (int)cipherKey[j]) % 26) + 'a');
                ciphertext[i] = plaintext[i];
            printf("%c", ciphertext[i]);
            // shift to the next in order
            // if reached the end of key word, restart from 0 index
            if (j == keyLength)
                j = 0;


int shift(char c)
    // convert character into (0-25) int value
    int key = 0;
    key = (toupper(c) - 'A');
    return key;

  • What are the check50 results? – Cliff B Apr 18 '19 at 14:22
  • I don't know how to do check50 correctly ?! – Ahmed-ElKodsh Apr 18 '19 at 19:57
  • Have you thoroughly tested your code? Is it working correctly? I just ran it with key aaa and plaintext test test and it failed badly. What test data did you run, what results did you expect and what did you get? I am trying to encourage you to create test data of your own to test various conditions, to determine what the results SHOULD be, and to compare them to actual results in order to debug the code yourself first. These are critical programming skills that you need to learn now, not later. – Cliff B Apr 18 '19 at 20:06
  • Thanks Cliff B for your effort, I've editted the code, but still dosen't work, could you check it now – Ahmed-ElKodsh Apr 19 '19 at 8:09
  • It dosen't work for the case of key with 'a' or multiples of 'a's.. could you help me with this? – Ahmed-ElKodsh Apr 19 '19 at 9:01

This code is simply too complex for the task. All of these issues are contributing to the problems that you are seeing.

When creating strings, remember to set the length to allow for the end of string marker, \0 at the end.

Kudos for creating and using the function shift() correctly. But you should learn functions toupper(), tolower() and related functions.

There's no need for special case code for a key of all a's, or for a key of length 1.

The central code block is a while loop. While loops are best used when the loop needs to terminate on a specific condition and the number of loops is unknown. A for loop is more appropriate for use when the number of passes is easily known, such as the length of a string.

Putting the usage else clause at the end of the code is a bad practice. It should be at the top of the code and not as an else clause. The usage check can be set up as a simple if statement. Further, best practice is NOT to set it up as an if/else statement block. Doing so requires that the entire bulk of the program is contained in the if statement block or the else statement block. By using a simple if statement, if the usage is incorrect, then an error message is printed and the program exits. If it doesn't exit, then the program code follows, without being contained in an else clause.

There's an issue with the code that handles non-alphas. It's simple enough, so I'll let you sort it out.

This should get you going. A lot of the code can be deleted. The central code block should be able to handle it.

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

  • Thanks for your care.. forgive me, I'm a beginner so I could be missing obvious things.. - I've editted the code above but still 4 out of 5 and there is something wrong.. could you guide me to it.. - Is that the right way for implying the "for" loop?.. - regarding checking the non-alphas, what I'm messing? How could I develop my code? any guidence with this would be appreciated.. – Ahmed-ElKodsh Apr 20 '19 at 13:24

You must log in to answer this question.

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