2
\$\begingroup\$

Given a string, count how many times a sequence of characters (in this code, the sequence is "ei") appears until the user types '#', then print the amount. I would like to know how to improve the code.

#include <stdio.h>
#include <inttypes.h>

int main(void)
{
    char ch, cur_ch, last_ch;
    cur_ch = '\0';
    uint16_t count = 0;

    puts("Enter a string (# to finish):");
    while ((ch = getchar()) != '#')
    {
        last_ch = cur_ch;
        cur_ch = ch;

        if (last_ch == 'e' && cur_ch == 'i')
            count++;
    }

    printf("There %s %" PRIu16 " %s.\n", count == 1 ? "is" : "are", count,
    count == 1 ? "ei" : "ei's");

    return 0;
}
\$\endgroup\$
3
\$\begingroup\$
  1. Missed detection of EOF: getchar() returns an int with values in the range of unsigned char and the negative value of EOF. Saving that result in a char loses distinctiveness should getchar() return EOF due to stdin becoming closed or input error.

    //char ch;
    //while ((ch = getchar()) != '#')
    int ch;
    while ((ch = getchar()) != '#' && ch != EOF)
    
  2. Why uint16_t? The input stream can be excessively long, and certainly more than 65,535. Suggest a wider type. (Good that code uses an unsigned integer.)

    // uint16_t count = 0;
    unsigned long long  = 0;
    
  3. Code could do this with 2 variables rather than 3.

    int ch;
    previous = EOF;
    
    while ((ch = getchar()) != '#' && ch != EOF) {
      if (previous == `e` && ch == `i') {
        count++;
      }
      previous = ch;
    }
    
  4. Recommend {}

    // if (last_ch == 'e' && cur_ch == 'i')
    //     count++;
    if (last_ch == 'e' && cur_ch == 'i') {
        count++;
    }
    
  5. Grammar: Code goes though work to print are vs. is yet does not make correct with ei's vs. ei.

|improve this answer|||||
\$\endgroup\$
2
\$\begingroup\$

Easier customization

In this code the sequence (ei) is specified inside the logic (the if statement. I suggest declaring the first and last char as constants at the top of your code to make changes and experimentation easier.

Another good solution is writing a function to check if two chars are consecutively present in the user input and supplyng ei as an argument in main.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Good point about the "constants at the top of your code". \$\endgroup\$ – chux - Reinstate Monica Apr 29 '16 at 18:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.