Mayank Goel Mayank Goel - 2 months ago 26
C Question

Vigenere Cipher logic error

#include<stdio.h>
#include<cs50.h>
#include<ctype.h>
#include<string.h>
int main(int argc, string argv[]){
int k,j,i=0,ch,pos;
bool apha=true;
string in = GetString();
int num = strlen(in);
for(int z=0;z<strlen(argv[1]);z++){
if(!isalpha(argv[1][z])){
apha=false;
}
}
if(argc!=2||!apha){
printf("Dude we only accept alphabets...");
return 1;
}
string key = argv[1];
int keylength = strlen(key);
for (i=0,j=0;i<num;i++,j++){
if(isupper(key[i])){
k=key[j%keylength]-'A';
}
if(islower(key[i])){
k=key[j%keylength]-'a';
}
if(isupper(in[i])){
pos=in[i]-'A';
ch = ((pos + k)%26) + 'A';
printf("%c",ch);
}
if(islower(in[i])){
pos=in[i]-'a';
ch = ((pos + k)%26) + 'a';
printf("%c",ch);
}
if(isspace(in[i])){
printf(" ");
}
if(ispunct(in[i])){
printf("%c",in[i]);
}
}
printf("\n");
}


Output condition checks:
:) 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 "xoqkj, yfd gfllp!\n"

:( encrypts "BaRFoo" as "CaQGon" using "BaZ" as keyword

\ expected output, but not "CaQEun\n"

:( encrypts "BARFOO" as "CAQGON" using "BAZ" as keyword

\ expected output, but not "CAQEON\n"

:( handles lack of argv[1]

\ expected output, not a prompt for input

:( handles argc > 2

\ expected output, not a prompt for input

:( rejects "Hax0r2" as keyword

\ expected output, not a prompt for input

What is wrong with my code? I have scrutinized the logic and the error seems to be in the way the key has been wrapped, though I could not find any errors. Where have I gone wrong?

Answer

There are several problems with your code:

You're error checking isn't correct. You check if(argc!=2||!apha) after you've already evaluated strlen(argv[1]) -- by then it's too late! Check the validity of argc before accessing argv and don't double up the argument count error and alphabetic key error, they're independent. Also, error messages should go to stderr, not stdout.

You're completely mishandling the key indexing. As @Bob__ noted, the indexing in this code:

if(isupper(key[i])){
    k=key[j%keylength]-'A';
}

needs to be consistent

if (isupper(key[j % keylength])) {
    k = key[j % keylength] - 'A';
}

But also, you're not incrementing j correctly, you have it tracking i:

for (i=0,j=0;i<num;i++,j++){

Instead, i should increment for every character in the input string, j should increment for every encryptable letter in the input string.

Reworking your code to fix the above errors and general style issues, we get something like:

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

int main(int argc, string argv[]) {

    if (argc != 2) {
        fprintf(stderr, "Please supply an encryption key.\n");
        return 1;
    }

    string key = argv[1];
    int key_length = strlen(key);
    bool is_alpha = true;

    for (int z = 0; z < key_length; z++) {
        if (!isalpha(key[z])) {
            is_alpha = false;
        }
    }

    if (!is_alpha) {
        fprintf(stderr, "Sorry, we only accept alphabetic keys.\n");
        return 1;
    }

    string in = GetString();
    size_t length = strlen(in);

    for (int i = 0, j = 0; i < length; i++) {

        if (isalpha(in[i])) {

            int ch, k = key[j++ % key_length];

            if (isupper(k)) {
                k -= 'A';
            } else {
                k -= 'a';
            }

            if (isupper(in[i])) {
                int pos = in[i] - 'A';
                ch = ((pos + k) % 26) + 'A';
            } else {
                int pos = in[i] - 'a';
                ch = ((pos + k) % 26) + 'a';
            }

            printf("%c", ch);
        } else if (isspace(in[i])) {
            printf(" ");
        } else if (ispunct(in[i])) {
            printf("%c", in[i]);
        }
    }

    printf("\n");

    return 0;
}

USAGE SIMULATION

> ./a.out baz
world, say hello!
xoqmd, rby gflkp!
>