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!
>
solved Vigenere Cipher logic error