2

Hello. I have a problem with a code I have created to decide if an input string is a palindrome or not (word is the same if read in wrong direction). The actual code in itself works as intended, however when I tried to put a loop on the whole code it started acting weird.

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

#define STRING_MAX 100

int isPalindrome();
char* removeNonAlpha();
char* strLowerCase();

int main()
{
    int x = 0;
    char inputString[STRING_MAX];
    char cont='y';

    do
    {
        printf("Hello. Please enter a word or sentence: ");
        fgets(inputString, STRING_MAX, stdin);

        printf("You entered: %s\n", inputString);

        //Remove all non alphabetical symbols from the string
        removeNonAlpha(inputString);
        printf("First we have to remove everything non-alphabetical: %s\n", inputString);

        //Make sure all the alphabetical symbols are lower case
        strLowerCase(inputString);
        printf("Then we have to make everything lower-case: %s\n", inputString);

        //After that, check if the string is a palindrome
        x = isPalindrome(inputString);

        if(x)
        {
            printf("The string you entered is a palindrome! :D\n");
        }
        else
        {
            printf("The string you entered is not a palindrome... :|\n");
        }
        printf("Would you like to enter another word or sentence? (y/n): ");
        cont = getchar();

        inputString[strlen(inputString)-1] = '\0';

    } while (cont == 'y');

    return 0;
}

int isPalindrome(char inputString[])
{
    int l = 0, r = strlen(inputString) - 1;

    if (l == r)
        return 1;

    while (r > l)
    {
        if (inputString[l++] != inputString[r--])
            return 0;
    }
    return 1;
}

char* removeNonAlpha(char inputString[])
{
    int i, j;

    for(i=0,j=0; i<strlen(inputString); i++,j++)
    {
        if (isalpha(inputString[i]))
            inputString[j] = inputString[i];
        else
            j--;
    }
    inputString[j] = '\0';

    return inputString;

}

char* strLowerCase(char inputString[])
{
    int i;

    for(i=0; i<strlen(inputString); i++)
        inputString[i] = tolower(inputString[i]);


    return inputString;
}

So what I expected was the code to run again and ask for another string to be entered, however it doesn't. I read around a bit about this and it looks like what is happening is what a newline (\n) is still in the input buffer and the fgets is skipped. I tried using a getchar() after the fgets to consume eventual newlines but it did not solve my problem. This is the output I am getting:

The output

Any ideas on how to solve this? I am new to programming and I would love to hear on how I could improve my code generally, although my main concern right now is fixing this loop issue.

EDIT: The only function from string.h that I am allowed to use is strlen().

JonathanDavidArndt
  • 2,518
  • 13
  • 37
  • 49
lemonslayer
  • 171
  • 10
  • Side issue: `for(i=0,j=0; i `inputString[i]` – chux - Reinstate Monica Dec 06 '17 at 20:41
  • 2
    Start by checking that `fgets` even *suceeded*. The return value of that function is there for a reason. Also, `fgets` consumes the newline, adding it to the end of the string-read, when it succeeds, so a followup `getchar` is unwarranted if all input is through `fgets`. – WhozCraig Dec 06 '17 at 20:42
  • @chux Thank you for the feedback! I am not super familiar with for-loops, what does this do exactly? Because I thought for-loops needed some form of condition, for example while i is less than the length of inputString. – lemonslayer Dec 06 '17 at 20:58
  • @WhozCraig Sorry if I sound nooby but how would I check that fgets suceeded and what would it mean it if has not suceeded? – lemonslayer Dec 06 '17 at 21:00
  • @user3121023 What would that change essentially? Because I used the `cont = getchar();` so the code would loop if the user input 'y'. Sorry if my question seems dumb I just really wanna understand what I am doing. – lemonslayer Dec 06 '17 at 21:03
  • You check by validating the result of the function indicates it succeeded (or not). What it ultimately *means*, if that result indicates failure, is that the content of `inputString` is not valid, and you should likely exit your loop, rather than marching on and assuming content in `inputString` is valid. It is a common mistake, sadly not just by beginners, and I often cite [Henry Spencer's Sixth Commandment](https://www.seebs.net/c/10com.html) among the reasons to be diligent about error states, and more specifically, checking for them judiciously. – WhozCraig Dec 06 '17 at 21:07
  • @WhozCraig Thank you for taking your time to explain that a little further to me! – lemonslayer Dec 06 '17 at 21:10
  • @MemErkanEktiren `inputString[i]` is the [condition](https://stackoverflow.com/questions/47683070/issue-with-fgets-inside-of-a-do-while-loop-in-c?noredirect=1#comment82326101_47683070). If the value is non-zero, continue the loop. Code could use the [WET](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself#DRY_vs_WET_solutions) `inputString[i] != '\0';`. `for(i=0; inputString[i]; i++)` is nice and clean. – chux - Reinstate Monica Dec 06 '17 at 21:15
  • @chux Yeah but what does it really mean that inputString[i] is the condition? Does it run through the whole string until it encounters the '\0'? In that case, wow. I never even thought of that. – lemonslayer Dec 06 '17 at 21:18
  • @MemErkanEktiren Another side tip: Do not use `inputString[strlen(inputString)-1] = '\0';` it is a hacker exploit (hacker enters the null character as the first character to be read). A classic one-liner alternative is `inputString[strcspn(inputString, "\n")] = '\0';`. See [ref](https://stackoverflow.com/a/28462221/2410359) and others. – chux - Reinstate Monica Dec 06 '17 at 21:22
  • @chux I forgot to mention, so it's my bad. However our teacher only allowed us to use the strlen function, no other string functions such as strcpy etc. Is there another way to do what I did without using any string functions from ? – lemonslayer Dec 06 '17 at 21:26
  • @MemErkanEktiren Yes, see others such as [this](https://stackoverflow.com/a/27729970/2410359) which uses `strlen()` – chux - Reinstate Monica Dec 06 '17 at 21:28
  • Press 'y', and then press enter. Problem solved. – autistic Dec 06 '17 at 21:30
  • @chux Thank you for your help, really appriciate it! – lemonslayer Dec 06 '17 at 21:32
  • @Sebivor That was the whole point, it did not work as intended. However it has been solved now, thank you! – lemonslayer Dec 06 '17 at 21:33

1 Answers1

2

Where the newline is getting left in the buffer is after the getchar call at the bottom of the loop. It only reads one character, so the newline (and any extra characters you may have entered) are still left. So when fgets is called on the next iteration, it reads just the newline.

You need to flush out the input buffer at the bottom of the loop by repeatedly calling getchar until you see a newline:

cont = getchar();

int nextchar;
do {
    nextchar = getchar();
} while (nextchar != '\n' && nextchar != EOF);
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Thank you so much! This worked. I was just wondering, does the getchar() in the dowhile loop actually consume the newline automatically and how does this work? Also what does the EOF stand for? – lemonslayer Dec 06 '17 at 20:52
  • 1
    @MemErkanEktiren It does not consume newlines automatically, it only reads one character at a time. So if you pressed "ABC[ENTER]". The first call to `getchar` returns `'A'`, the next call `'B'`, the next `'C'`, and the next `'\n'`. `EOF` is a constant that indicates the end of the stream was reached, and the reason `getchar` returns an `int`. – dbush Dec 06 '17 at 20:55
  • 1
    You might find `fread` to be more expressive for this "read and test a character" kind of scenario. `char c; while (fread(&c, 1, 1, file) && c != '\n');`; it also seems much more difficult for beginners to get wrong. or you could use `scanf` as your loop: `scanf("%*[^\n]"); getchar();` ... in fact, the whole line can be *scanned*, user input obtained in the same call: `int answer = 0; fscanf("%*[Yy]%n%*[^\n]", &answer); getchar();`... – autistic Dec 06 '17 at 21:37
  • 1
    Don't mind me... I'm just offering more expressive alternatives. I don't disagree with the solution; providing `cont` is declared as`int` (i.e. like `nextchar`), this is a good answer. I just figured it'd be best to have the other alternatives in the same place. – autistic Dec 06 '17 at 21:40