0

I'm a beginner programmer trying to learn C. Currently I'm taking a class and had a project assigned which I managed to finish pretty quickly, at least the main part of it. I had some trouble coding around the main() if functions though, because I started using some new functions (that is, fgets and strncmp). Now, my code works in my compiler, but not in any of the online compilers. So I'm wondering if I did something wrong with it, or if there is any way I can improve it.

Any help or contribution is appreciated, thanks!

Below is the code, the encrypt and decrypt functions are the first two functions before the main, where I believe most of the messy shortcut-code might be.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char * Encrypt(char sentence[])
{
int primes[12] = {1,2,3,5,7,11,13,17,19,23,29,31};
int x = 0;
int counter = 0;
int ispositive = 1;

 while(sentence[x] != 0)
    {
    if (counter == 0)
        {
        ispositive = 1;
        }
    else if(counter == 11)
        {
        ispositive = 0;
        }

    if (ispositive == 1)
        {
        sentence[x] = sentence[x] + primes[counter];
        counter++;
        }
    else if (ispositive == 0)
        {
        sentence[x] = sentence[x] + primes[counter];
        counter--;
        }
    x++;
    }

    return sentence;
}

char * Decrypt(char sentence[])
{
int primes[12] = {1,2,3,5,7,11,13,17,19,23,29,31};
int x = 0;
int counter = 0;
int ispositive = 1;

 while(sentence[x] != 0)
    {
    if (counter == 0)
        {
        ispositive = 1;
        }
    else if(counter == 11)
        {
        ispositive = 0;
        }

    if (ispositive == 1)
        {
        sentence[x] = sentence[x] - primes[counter];
        counter++;
        }
    else if (ispositive == 0)
        {
        sentence[x] = sentence[x] - primes[counter];
        counter--;
        }
    x++;
    }

    return sentence;
}

int main()
    {
    char message[100];
    char input[7];
    char *p;
    int c;
    int condition = 1;

    while(condition == 1)
    {

    printf("Would you like to Encrypt or Decrypt a message? (Type TurnOff to end the program) \n \n");

    fgets(input,7, stdin);

    fflush(stdin);

        if (!strncmp(input,"Encrypt",strlen(input)))
            {

            printf("\n \n Enter the message you want to Encrypt below: \n \n");

            fgets(message, 100, stdin);

            Encrypt(message);

            printf("\n Your encrypted message is: ");

            printf("%s", message);

            fflush(stdin);

            printf("\n \n");

            }
         else if (!strncmp(input,"Decrypt",strlen(input)))
         {
            printf("\n \n Enter the message you want to Decrypt below: \n \n");

            fgets(message, 100, stdin);

            Decrypt(message);

            printf("\n Your Decrypted message is: ");

            printf("%s", message);

            fflush(stdin);

            printf("\n \n");
         }
         else if (!strncmp(input,"TurnOff",strlen(input)))
         {
            printf("\n \n Thank you for using the program! \n \n");
            condition = 0;
         }
         else
         {
             printf("That's not a valid input \n \n");
         }
    }


    }
  • For one, the memory size of your `char*` isn't the number of characters but that number times `sizeof(char)`. I would change `fgets(input, 7, stdin)` to `fgets(input, sizeof(input), stdin)`. Do the same for the other `fgets` statements. – Arc676 Oct 24 '15 at 07:45
  • 1
    See http://stackoverflow.com/questions/2979209/using-fflushstdin – chux - Reinstate Monica Oct 24 '15 at 07:46
  • Recommend to do somthing when `stdin` closes like `if (fgets(input, sizeof input, stdin) == NULL) return -1;` – chux - Reinstate Monica Oct 24 '15 at 07:48
  • 1
    `strcmp(input,"TurnOff")` works fine. `strncmp(x, string_literal, strlen(x))` has little value, unless you like typing. – chux - Reinstate Monica Oct 24 '15 at 07:50
  • 1
    `char input[7];` too small for entering in `"Encrypt\n\0"`. Then `if (!strncmp(input,"Encrypt",strlen(input)))` will never be true - pesky `'\n'`. GTG – chux - Reinstate Monica Oct 24 '15 at 07:52
  • Please format your code properly - not just for the benefit of your readers, but also for yourself - it makes reading the code and spotting bugs much easier. – Paul R Oct 24 '15 at 07:53

2 Answers2

1

After the printf you doing fflush(stdin) instead of you have to do fflush(stdout). Because you are printing the output. The output is printed in stdout. So, you have to flush the stdout buffer not stdin buffer.

You can use the strcmp instead of strncmp. Because in here you are comparing the hole character in the input array. So, the strcmp is enough.

strcmp(input, "Encrypt").

The strcmp or strncmp function get the input in array upto a null or the size of the string you are declared.

The size for the input array is too few.

lets take the input is like below.

Encrypt\n sureshkumar\n

In here you first fgets in main function reads the upto "Encrypt" it does not skip the '\n'.

The '\n' is readed form another fgets. So, it does not get the encrypt message "sureshkumar".

So, you have to modify you code. You will increase the size for the input array.

And check the condition like below.

if(strcmp(input, "Encrypt\n") == 0)
{
/*
 You will do what you want
*/
}

You can use the above way or you can read the input and overwrite the '\n' to '\0' in the input array and compare as it is you before done. But you have to use the strcmp. Because the array size is incremented.

This is the right way for using the fgets. Use of fgets is to read upto new line.

You have to use the null character for the character array. Because this is necessary for the character arrays.

0

Your initiative towards using strcmp() and fgets() is good, though, it requires following understanding:
1. fgets() writes atmost size-1 characters into buffer and then terminates with '\0'. In your case,

fgets(input,7, stdin);

You gave input "Encrypt"/"Decrypt"/"TurnOff"
but
'input' buffer got data as "Encryp"/"Decryp"/"TurnOf"
because of size=7 (only (7-1)=6 characters being read, last position reserved for '\0' character by fgets()).

  1. Your strncmp() calls will work correctly with your current code, since for strncmp(), length to compare

    n = strlen(input) = 6;

6 characters are matching fine in all three cases of "Encrypt"/"Decrypt"/"TurnOff".

Summary is that your current code will work fine, But your actual intention is violated. You actually wanted to read and compare full length of option string.

EDIT DONE : Modifications suggested:

#define SIZE 9   <-- EDIT : Change done here, instead of 7, size = 9 is used
                            to allow reading '\n' so that it does not affect
                            fgets() read in successive iteration
char input[SIZE];

fgets(input, SIZE, stdin); // read str is e.g. "Encrypt\n"
input[SIZE-2] = '\0'; // To replace '\n' with '\0'

Similarly, you need to be careful when reading into 'message' array using fgets().

cm161
  • 472
  • 3
  • 6
  • 1) `input[8]` is insufficient for user entry of `"Encrypt\n"`. 2) Little need for a minimalistic input array size: Recommend 80. – chux - Reinstate Monica Oct 24 '15 at 15:12
  • Hi chux, '\n' is read into the buffer only if (size-1) length can accomodate it (where 'size' is input length to fgets() call). Otherwise, it is not read into the buffer as reading into buffer terminated at (size-1). In such case, '\n' is only indicator for fgets() to come out of input reading. I have confirmed this through code verification. So, For this question, size = 8 will be sufficient to read full length of option string and result in null terminated string. Thanks. – cm161 Oct 25 '15 at 04:32
  • True that size 8 is sufficient for `"Encrypt"`, but that is not the issue. The `'\n'` remains unread in `stdin` to mess up the next call to `fgets()`. – chux - Reinstate Monica Oct 25 '15 at 05:25
  • 1
    Ok, this is good point. I will edit my answer to use SIZE=9. Thanks. – cm161 Oct 25 '15 at 06:31