Oscar Rodriguez
Oscar Rodriguez

Reputation: 3

Am I using strncmp and fgets in the right way?

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");
         }
    }


    }

Upvotes: 0

Views: 300

Answers (2)

cm161
cm161

Reputation: 510

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().

Upvotes: 0

user5076313
user5076313

Reputation:

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.

Upvotes: 1

Related Questions