Thread: Caesar Cipher in C

  1. #1
    Registered User
    Join Date
    Nov 2018
    Posts
    21

    Caesar Cipher in C

    Hi,
    I was wondering if anyone could review my code and suggest how to improve or optimize this code. I'm very new to C and programming.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <ctype.h>
    
    
    int euclideanMod(int a, int b) {
      int mod = a % b;
      if (mod < 0) mod = (b < 0) ? mod - b : mod + b;
      return mod;
    }
    
    
    int str_indexOf(const char* str, char c)
    {
        int index = -1;
        char* pos = strchr(str, c);
        if (pos != NULL) index = pos - str;
        return index;
    }
    
    
    void getChoice(const char* msg, char* c)
    {
        do
        {
            printf("%s", msg);
            scanf(" %c", c);
            if (*c == 'd' || *c == 'e') break;
            printf("wrong choice selected!\n");
        } while (1);
    }
    
    
    void getKey(const char* msg, int* k)
    {
        do
        {
            printf("Please select secret key to %s: ", msg);
            scanf(" %d", k);
            if (*k >= 1 && *k <= 26) break;
            printf("key length must be 1 to 26\n");
        } while (1);
    }
    
    
    char shiftChar(int key, const char* alp_Arr, char c, _Bool encrypt)
    {
        char e_C = c;
        _Bool is_upper = isupper(c);
        if (is_upper) e_C += 32;
        int index = str_indexOf(alp_Arr, e_C);
        if (index != -1)
        {
            if (encrypt)
            {
                e_C = alp_Arr[euclideanMod((index + key),  26)];
            }
            else e_C = alp_Arr[euclideanMod((index - key),  26)];
        }
        if (is_upper) e_C -= 32;
        return e_C;
    }
    
    
    void cipher(const char* msg, char* outMsg, size_t buff_size, int key, const char* alp_Arr, _Bool encOrDec)
    {
        char enc_msg[buff_size];
        int msg_len = (int)strlen(msg);
        for (int i = 0; i < msg_len; i++) enc_msg[i] = shiftChar(key, alp_Arr, msg[i], encOrDec);
        strcpy(outMsg, enc_msg);
    }
    
    int main()
    {
        char msg[300], out[300];
        _Bool enOrDe;
        int k;
        char choice;
        getChoice("Please select \"e\" for encrypt or \"d\" for decrypt: ", &choice);
        if (choice == 'e')
        {
            getKey("encrypt", &k);
            enOrDe = 1;
        }
        else
        {
            getKey("decrypt", &k);
            enOrDe = 0;
        }
        getchar(); // removes trailing '\n' from last scanf() by getKey()
        printf("please enter your message: ");
        fgets(msg, sizeof(msg), stdin);
        char* pos;
        if ((pos=strchr(msg, '\n')) != NULL) *pos = '\0';
        char alp[26];
        for (int i = 0; i < 26; i++) alp[i] = (char)((97+ i));
        cipher(msg, out, 300, k, alp, enOrDe);
        printf("%s\n", out);
        return 0;
    }
    Last edited by flash13; 12-30-2018 at 08:34 PM.

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    3,653
    Code:
    void getChoice(const char* msg, char* c)
    {
        do
        {
            printf("%s", msg);
            scanf(" %c", c);
            if (*c == 'd' || *c == 'e') break;
            printf("wrong choice selected!\n");
        } while (1);
    }
    I would add a blank line under the if statement; this is a stylistic choice; I read the next line as being part of the if by mistake; and,
    edit the code below to show how I would avoid this dumb mistake that others might make.
    Code:
            if (*c == 'd' || *c == 'e') break;
    
            printf("wrong choice selected!\n");


    Also, I would have getChoice return the char instead of passing by address. But, this is a stylistic choice where either is correct.

    Tim S.
    Last edited by stahta01; 12-30-2018 at 08:22 PM. Reason: edit code change because I read it wrong
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  3. #3
    Registered User
    Join Date
    Nov 2018
    Posts
    21
    Thanks. I like pointers, as you mentioned it's a choice of style.

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,070
    Quote Originally Posted by stahta01
    I would add a blank line under the if statement; this is a stylistic choice; I read the next line as being part of the if by mistake
    Instead of doing that, always use braces for these if statements and loops, even if the body of the if statement is only one statement. That is:
    Code:
    if (*c == 'd' || *c == 'e')
    {
        break;
    }
    This avoids variants of the goto fail bug, which basically is a case of formatting tricking both author and reviewer. Adding a blank line could help visually, but then blank lines are commonly used to separate logical portions of code, whereas it is quite possible that the code following an if statement should be grouped with it.

    By the way, note that this is a variable length array:
    Code:
    char enc_msg[buff_size];
    This was made part of the C standard back in 1999, but apparently adoption was poor so they reversed course and now it is merely an optional part of standard C in the 2011 edition of the C standard.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Nov 2018
    Posts
    21
    thanks. I'm aware of that. But I've been writing this way in C++, and C# as well, never had problems so far. And read about VLA before. But thanks again!

  6. #6
    Registered User
    Join Date
    Dec 2018
    Posts
    38
    Quote Originally Posted by flash13 View Post
    Thanks. I like pointers, as you mentioned it's a choice of style.
    It isn't a stylistic choice at all. It is objectively better to return the value when possible. It allows the function to be used more flexibly. E.g.,
    Code:
    switch (getChoice())
    {
    case 'd':
        //...
        break;
    case 'e':
        //...
        break;
    }
    Other points about getChoice:
    Always choose while over do/while when applicable.
    Passing in msg is not very useful here since the possible entries (d or e) are hardcoded. The message may as well be hardcoded too. Alternativly, you could pass in a string of the possible choices as well as the message.
    Finally, it should clean up after itself instead of leaving the newline in the input buffer. A function like the following is useful:
    Code:
    void clear_input() {
        for (int c; (c = getchar()) != '\n' && c != EOF; )
            ;
    }
    There are many other points I could make but you don't seem to be taking other peoples' advice, so I feel I may be wasting my time as well.

  7. #7
    Registered User
    Join Date
    Nov 2018
    Posts
    21
    you misunderstood me. I'm here to be reviewed, and learn, but I do have the right to express my opinion. For me pointers seem interesting, that's why I try to use them where I can. If returning value was the only objective I should rather stick with C#, Java or Python. But plz go ahead to point out other mistake or advice that may have on my code, it won't be a waste of time for me for sure. Thanks.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. I need help on what I think is a Caesar cipher......
    By CaesarCipher in forum C++ Programming
    Replies: 8
    Last Post: 11-28-2011, 04:13 PM
  2. Caesar Cipher help!!
    By darshan10 in forum C Programming
    Replies: 6
    Last Post: 10-19-2011, 04:58 PM
  3. Caesar Cipher
    By dldsob in forum C++ Programming
    Replies: 7
    Last Post: 07-06-2009, 06:06 PM
  4. Another Caesar Cipher
    By Swordsman in forum C++ Programming
    Replies: 6
    Last Post: 09-07-2007, 08:56 AM
  5. Help with Caesar cipher
    By jcmichman in forum C++ Programming
    Replies: 1
    Last Post: 04-05-2005, 10:50 AM

Tags for this Thread