I'm working on a simple Ceasar Cipher encryption program in C. I am trying to make it simpler for my son and I to decipher so I put these rules: characters other than letters are unchanged while lowercase and uppercase will remain in their current case (so a Y incremented by 3 would go back to a B). The key and phrase will just be piped using a separate file (my secret!) though a sample of it's contents would be:

This line of text will be encrypted.

I wanted to buffer it character by character using getchar and putchar so I don't have to bother with an array whose length will always be unknown. How can I check if the character is either an upper or lower case letter and increment it by they given key, while keeping in line with the previous rules. Should putchar be inside the loop to buffer it? Here is my current code:

#include <stdio.h>  
int main() { 

   int shift; 
   char msgIn, msgOut;

   // space after to keep is from terminating immediately
   scanf("%d ", &shift);
   msgIn = getchar();

   //increment current character and output until newline
   while (msgIn != '\n') {
     //check for upper or lower, else do nothing
     if((msgIn >= 'A') && (msgIn <= 'Z')) {
        msgOut = ((msgIn - 'A') + shift) % 26 + 'A'; //increment current character, not sure how to handle this better 
     //checking for lower case 
     else if((msgIn >= 'a') && (msgIn <= 'z')) {
        msgOut = ((msgIn - 'a') + shift) % 26 + 'a'; //increment current character 

     putchar(msgOut); //output incremented character

   return 0; 

-------------- Revised code

    do {
        msgIn = getchar();

        if((msgIn >= 'A') && (msgIn <= 'Z')) {
            putchar(((msgIn - 'A') + shift) % 26 + 'A'); 
        else if((msgIn >= 'a') && (msgIn <= 'z')) {
            putchar(((msgIn - 'a') + shift) % 26 + 'a'); 
    } while (msgIn != '\n') 

I am thinking this code handles the input, checking, incrementing, and output better. LMK what you think.

Edited by Jophis

2 Years
Discussion Span
Last Post by loserspearl

In both cases you have the following problems

  1. scanf("%d ", &shift); the space after the %d is superfluous and could cause issues, remove it.

  2. scanf will likely leave the '\n' (newline) after the number in the input stream which means in both cases the first character read by getchar will be a '\n' ending your program. Better to treat '\n' as just another non-translateable character and use a sentinel character, say '~' as the EOT (end of text) marker.

  3. You state that all non letter characters will be passed through untranslated, i.e. 0-9, ' ' etc, however what your code does in all cases is not output them at all, you need an else at the end of your if ... else if.

To my mind you orignal code is better than your revised code. Your revised code has multiple putchar statements, you are mixing logic and output. In the original code the decision is made what needs outputing, then it is output. it splits the operation into 2 distinct steps which is good because it then makes changing either step more easy to do independantly of the other step.

Edited by Banfa


The piped file doesn't have a space between the key and the message, only a newline. The lack of space in the scanf was causing the program to terminate prematurely, I think it is my compiler. There could even possibly be NOT a newline, because my son is trying to break the program by piping nonstandard file types. I make my file and he is changing the extension to mess with me; I changed the txt file so the contents can't be edited or viewed, but windows explorer will still allow entension change. After some testing I did add an else of putchar(msgIn), I usually do it the other way but I'm still testing different buffers on other programs.

This article has been dead for over six months. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.