Detecting a combination of characters from input

Question

Write a program that reads input up to # and reports the number of times that the sequence ei occurs.

Is this a decent code?

#include <stdio.h>

int main(void)

{
    int index = 0, combinationTimes = 0, total = 0;
    char userInput;
    char wordChar[index];

    printf("please enter your input:\n");

    while ((userInput = getchar()) != '#')
    {
        if (userInput == '\n')
            continue;

        wordChar[index] = userInput;
        index++;
        total++;
    }

    for (index = 1; index < total; index++)
    {
        if (wordChar[index] == 'i')
        {
            if (wordChar[--index] == 'e')
            {
            combinationTimes++;
            ++index;
            }
        }
    }

    printf("number of combination is: %d", combinationTimes);

    return 0;
}

Answer 1

Here are some inputs on command line which break your code:

dan@albatross  $ gcc -Wall f.c -o f
dan@albatross  $ ./f
please enter your input:
bfiqwb23b r9pu3h2ru23r
9aisdbfuiasdf
adsf#asdf
#
#
#
    adsfadsfasdf
34324!
^C

I assume your program is supposed to terminate at the first #, but notice here that it does not. I have to kill the program to stop it. The shows there is a problem with one of your loops. By seeing the manipulation of the index variable in the second loop, I think it's the problem.

Code Review

First: you do not need to have the line index++ inside the loop. Use the single variable total instead. This is a minor thing.

Instead of having a nested if-statement, use an and conditional. And stop decreasing the index variable - that's what causing all those problems. Try this:

for (index = 1; index < total; index++)
    {   
        if (wordChar[index] == 'i' && wordChar[index - 1] == 'e')
            combinationTimes++;
    } 

However, that is still inefficient code, because you should be processing the input as it is given, not processing it after the last hash. Notice that you are doing length_of_input work at the end, when you could be doing constant work at each step, which is computationally less noticeable to a user. In addition, the array has length_of_input memory overhead, which seems unnecessary here. Consider writing something like this instead:

#define FALSE 0
#define TRUE 1

int main(void)
{
    int combinationTimes = 0;
    char userInput;
    short int last_e = FALSE;

    printf("please enter your input:\n");

    while ((userInput = getchar()) != '#')
    {   
        if (userInput == 'e')
        {   
            last_e = TRUE;
        }   
        else if (userInput == 'i' && last_e) 
        {
            combinationTimes++;
            last_e = FALSE;
        }
        else
        {
            //this is important, otherwise combinations like 'ite' are counted
            last_e = FALSE;
        }
    }   

    printf("Number of combos: %d\n", combinationTimes);
    return 0;
}

Here are some key features:

• I removed your array, so I have no significant memory overhead. Each char is processed and discarded, one at a time

• I remove your variables total and index

• I process the event you care about inside the first for loop, as you read the character

• I create boolean variables with compiler flags

This code also has a lot of room for improvement, but that's left as an exercise...