Vigenere's Cipher in C I have updated the code and the program now works as it should. If you could critique my code that would be great. I am new to C so I guess I will have made some rookie error :) 

#include &lt;stdio.h&gt;
#include &lt;stdlib.h&gt;
#include &lt;string.h&gt;
#include &lt;ctype.h&gt;
#include &lt;stdbool.h&gt;

// Function to check if alphabet characters.
int alphaCheck(char * argv[]) {
  int length = strlen(argv[1]), n;
  for (n = 0; n &lt; length; n++) {
    if (!isalpha(argv[1][n])) {
      printf("Characters 'A-Z' for Keyword.\n");
      return 2;
    }
  }
}

int main(int argc, char * keyWord[]) {
  int cipher[64], i, j, k;
  char message[128];
  int keyCount = 0;
  int p = strlen(keyWord[1]);

  // Validation - using alphaCheck Function
  if (argc != 2 || alphaCheck(keyWord) == 2) {
    return 0;
  }

  // For loop to convert to upper and set value ie A = 0 B = 1
  for (i = 0, j = strlen(keyWord[1]); i &lt; j; i++) {
    cipher[i] = (toupper(keyWord[1][i]) - 65);
  }

  // Prompt the user for the message to encrypt
  printf("Enter your secret message: ");
  fgets(message, 128, stdin);

  for (i = 0, k = strlen(message); i &lt; k; i++) {
    if (isupper(message[i])) {
      char c = (message[i] - 65 + cipher[keyCount])  % 26 + 65;
      printf("%c", c);
      keyCount++;
    }
    else if (islower(message[i])) {
      char d = (message[i] - 97 + cipher[keyCount]) % 26 + 97;
      printf("%c", d);
      keyCount++;
    }
    else {
      printf("%c\n", message[i]);
    }
    if (keyCount &gt;= p) {
      keyCount = 0;
    }
  }
}
i++ or ++i?

Using i++ is fine here, but also read this discussion:

https://stackoverflow.com/questions/4261708/i-or-i-in-for-loops

fgets(message, 128, stdin);

How about fget(message, sizeof(message), stdin) -- makes it easier to change the size of the message buffer.

int main(int argc, char * keyWord[])

Most people are used to seeing:

int main(int argc, char* argv[])


int alphaCheck(char * argv[])

How about:

int alphaCheck(char *str) {
  for ( ;*str; ++str) {
    if (!isalpha(*str)) {
      ...
    }
  }
  return 0;   // You don't have this return.
}


First of all, this makes alphaCheck usable on any string, not just argv[1]. Also you avoid computing the length of the string. Moreover, this is the idiomatic way to iterate over a null-terminated string in C.

Finally - make sure you always return a value since you've declared this function returns an int.

for (i = 0, j = strlen(keyWord[1]); i &lt; j; i++) {

You are modifying cipher[i] but are you checking that i &lt; 64? What if keyWord[1] has length > 64?

keyWord[1]

You reference this array element many times in the program. Perhaps you should assign a meaningful variable name to it, e.g.:

char* secret = keyWord[1];


It will make you program more readable.

printf("%c\n", message[i]);

Do you want to always print a \n here? I would think you only want to print an extra \n at the end of the message.

the main loop

You main loop has this structure:

for each character c in the message:
  let e = encrypt c using the secret key
  printf("%c", e)
  advance pointers, counters, etc.
printf("\n")


so it would be nice if the code could match this as much as possible.

How about:

int i;
for (char *p = message, i = 0; *p; ++p, ++i) {
  char e = ...encrypt (*p) using secret, strlen(secret) and i...
  printf("%c", e);
}
printf("\n");


Now there is only one place where the encrypted character is printed out - instead of three difference places.

You can implement this via a helper function:

char encrypt(char* secret, int secret_len, char c, int i) {
  // return how c is encrypted when at position i in the message.
  if (isupper(c)) {
    return ...
  } else if (islower(c)) {
    return ...
  } else {
    return c;
  }
}


Hint: The expression i % secret_len might come in handy here.