patterncMinor
Detecting a combination of characters from input
Viewed 0 times
combinationinputcharactersfromdetecting
Problem
Write a program that reads input up to # and reports the number of times that
the sequence ei occurs.
Is this a decent code?
the sequence ei occurs.
Is this a decent code?
#include
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;
}Solution
Here are some inputs on command line which break your code:
I assume your program is supposed to terminate at the first
Code Review
First: you do not need to have the line
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:
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
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...
dan@albatross $ gcc -Wall f.c -o f
dan@albatross $ ./f
please enter your input:
bfiqwb23b r9pu3h2ru23r
9aisdbfuiasdf
adsf#asdf
#
#
#
adsfadsfasdf
34324!
^CI 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...
Code Snippets
dan@albatross $ gcc -Wall f.c -o f
dan@albatross $ ./f
please enter your input:
bfiqwb23b r9pu3h2ru23r
9aisdbfuiasdf
adsf#asdf
#
#
#
adsfadsfasdf
34324!
^Cfor (index = 1; index < total; index++)
{
if (wordChar[index] == 'i' && wordChar[index - 1] == 'e')
combinationTimes++;
}#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;
}Context
StackExchange Code Review Q#21010, answer score: 4
Revisions (0)
No revisions yet.