Robustify code dealing with input

Collapse
This topic is closed.
X
X
 
  • Time
  • Show
Clear All
new posts
  • Eric Lilja

    #1

    Robustify code dealing with input

    Hello, consider the following complete program:

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

    static int has_char(const char *, const char);
    static void handle_guess(co nst char *, char *, const char);

    int
    main()
    {
    const char * words[] =
    {
    "hello", "usenet", "breakfast"
    };
    const char *actual = NULL;
    char *current = NULL;

    srand(time(NULL ));

    actual = words[rand() % 3];
    current = calloc(strlen(a ctual) + 1, sizeof(char));
    assert(current) ;
    memset(current, '?', strlen(actual)) ;

    while(1)
    {
    char c = '\0';

    if(!has_char(cu rrent, '?'))
    break;

    printf("The word is: %s\n", current);

    printf("Type a letter: ");

    c = getc(stdin);
    getc(stdin);

    handle_guess(ac tual, current, tolower(c));
    }

    printf("Congrat ulations! The word was: %s\n", current);

    free(current);

    return EXIT_SUCCESS;
    }

    static int
    has_char(const char *str, const char c)
    {
    unsigned short i = 0;

    for(i = 0; i < strlen(str); ++i)
    if(str[i] == c)
    return 1;

    return 0;
    }

    static int
    find_position(c onst char *str, const char c, const unsigned short start)
    {
    unsigned short i = start;

    if(start > strlen(str))
    return -1;

    for(i = start; i < strlen(str); ++i)
    if(str[i] == c)
    return i;

    return -1;
    }

    static void
    handle_guess(co nst char *actual, char *current, const char c)
    {
    int position = 0;

    assert(strlen(a ctual) == strlen(current) );

    if((position = find_position(a ctual, c, 0)) == -1)
    {
    printf("No match for the letter %c, sorry.\n", c);

    return;
    }

    if(current[position] != '?')
    {
    printf("You already guessed the letter %c\n", c);

    return;
    }

    current[position] = c;

    while((position = find_position(a ctual, c, position + 1)) != -1)
    {
    assert(current[position] == '?');

    current[position] = c;
    }
    }


    It maintains a list of words and randomly selects a word from the list when
    the program is started. The word is printed to the screen, but all
    characters have been replaced with a '?'. The user is asked to guess a
    letter in the word, and he if he guesses correctly the '?'s hiding that
    particular letter is replaced with the letter itself. This continues until
    the entire word has been revealed.

    My problem is that the code for obtaining user input isn't very robust.
    Say the user presses <tab>the_letter <enter>, it doesn't work anymore.
    Whatever I type next, the character passed to handle_guess() is "messed up".
    Here's a screen dump from such a session:
    $ ./guess_word.exe
    The word is: ?????????
    Type a letter: b
    The word is: b????????
    Type a letter: a
    The word is: b??a??a??
    Type a letter: s <---- Here I typed <tab>s<Enter>
    No match for the letter , sorry.
    The word is: b??a??a??
    Type a letter: i
    No match for the letter
    , sorry.
    The word is: b??a??a??
    Type a letter: t
    No match for the letter
    , sorry.

    As you can see, it remains in a broken state because of that <tab>.
    How can improve the code for dealing with user input so I can counter this
    problem?

    Any other comments regarding the code are welcome also.

    / E


  • pete

    #2
    Re: Robustify code dealing with input

    Eric Lilja wrote:[color=blue]
    >
    > Hello, consider the following complete program:
    >
    > #include <assert.h>
    > #include <ctype.h>
    > #include <stdlib.h>
    > #include <stdio.h>
    > #include <string.h>
    > #include <time.h>
    >
    > static int has_char(const char *, const char);
    > static void handle_guess(co nst char *, char *, const char);
    >
    > int
    > main()
    > {
    > const char * words[] =
    > {
    > "hello", "usenet", "breakfast"
    > };
    > const char *actual = NULL;
    > char *current = NULL;
    >
    > srand(time(NULL ));
    >
    > actual = words[rand() % 3];
    > current = calloc(strlen(a ctual) + 1, sizeof(char));
    > assert(current) ;
    > memset(current, '?', strlen(actual)) ;
    >
    > while(1)
    > {
    > char c = '\0';
    >
    > if(!has_char(cu rrent, '?'))
    > break;
    >
    > printf("The word is: %s\n", current);
    >
    > printf("Type a letter: ");
    >
    > c = getc(stdin);
    > getc(stdin);
    >
    > handle_guess(ac tual, current, tolower(c));
    > }
    >
    > printf("Congrat ulations! The word was: %s\n", current);
    >
    > free(current);
    >
    > return EXIT_SUCCESS;
    > }
    >
    > static int
    > has_char(const char *str, const char c)
    > {
    > unsigned short i = 0;
    >
    > for(i = 0; i < strlen(str); ++i)
    > if(str[i] == c)
    > return 1;
    >
    > return 0;
    > }
    >
    > static int
    > find_position(c onst char *str, const char c, const unsigned short start)
    > {
    > unsigned short i = start;
    >
    > if(start > strlen(str))
    > return -1;
    >
    > for(i = start; i < strlen(str); ++i)
    > if(str[i] == c)
    > return i;
    >
    > return -1;
    > }
    >
    > static void
    > handle_guess(co nst char *actual, char *current, const char c)
    > {
    > int position = 0;
    >
    > assert(strlen(a ctual) == strlen(current) );
    >
    > if((position = find_position(a ctual, c, 0)) == -1)
    > {
    > printf("No match for the letter %c, sorry.\n", c);
    >
    > return;
    > }
    >
    > if(current[position] != '?')
    > {
    > printf("You already guessed the letter %c\n", c);
    >
    > return;
    > }
    >
    > current[position] = c;
    >
    > while((position = find_position(a ctual, c, position + 1)) != -1)
    > {
    > assert(current[position] == '?');
    >
    > current[position] = c;
    > }
    > }
    >
    > It maintains a list of words and randomly selects a word from the list when
    > the program is started. The word is printed to the screen, but all
    > characters have been replaced with a '?'. The user is asked to guess a
    > letter in the word, and he if he guesses correctly the '?'s hiding that
    > particular letter is replaced with the letter itself. This continues until
    > the entire word has been revealed.
    >
    > My problem is that the code for obtaining user input isn't very robust.
    > Say the user presses <tab>the_letter <enter>, it doesn't work anymore.
    > Whatever I type next, the character passed to handle_guess() is "messed up".
    > Here's a screen dump from such a session:
    > $ ./guess_word.exe
    > The word is: ?????????
    > Type a letter: b
    > The word is: b????????
    > Type a letter: a
    > The word is: b??a??a??
    > Type a letter: s <---- Here I typed <tab>s<Enter>
    > No match for the letter , sorry.
    > The word is: b??a??a??
    > Type a letter: i
    > No match for the letter
    > , sorry.
    > The word is: b??a??a??
    > Type a letter: t
    > No match for the letter
    > , sorry.
    >
    > As you can see, it remains in a broken state because of that <tab>.
    > How can improve the code for dealing with user input so I can counter this
    > problem?
    >
    > Any other comments regarding the code are welcome also.
    >
    > / E[/color]

    /* BEGIN new.c */

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

    static void
    handle_guess(co nst char *, char *, const int);

    int
    main(void)
    {
    const char * words[] = {
    "hello", "usenet", "breakfast"
    };
    const char *actual = NULL;
    char *current = NULL;

    srand(time(NULL ));
    actual = words[rand() % 3];
    current = calloc(strlen(a ctual) + 1, sizeof(char));
    if (current == NULL) {
    exit(EXIT_FAILU RE);
    }
    memset(current, '?', strlen(actual)) ;
    while (strchr(current , '?') != NULL) {
    int c;

    printf("The word is: %s\n", current);
    printf("Type a letter: ");
    fflush(stdout);
    do {
    c = getc(stdin);
    } while (!isalpha(c));
    getc(stdin);
    handle_guess(ac tual, current, tolower(c));
    }
    printf("Congrat ulations! The word was: %s\n", current);
    free(current);
    return EXIT_SUCCESS;
    }

    static void
    handle_guess(co nst char *actual, char *current, const int c)
    {
    char *position;

    position = strchr(actual, c);
    if (position == NULL) {
    printf("No match for the letter %c, sorry.\n", c);
    return;
    }
    if (current[position - actual] != '?') {
    printf("You already guessed the letter %c\n", c);
    return;
    }
    do {
    current[position - actual] = (char)c;
    position = strchr(position + 1, c);
    } while (position != NULL);
    }

    /* END new.c */

    --
    pete

    Comment

    • Barry Schwarz

      #3
      Re: Robustify code dealing with input

      On Sat, 4 Jun 2005 04:09:37 +0200, "Eric Lilja"
      <mindcooler_thi sshouldberemove d@gmail.com> wrote:


      snip
      [color=blue]
      > printf("Type a letter: ");
      >
      > c = getc(stdin);[/color]

      Hopefully a letter.
      [color=blue]
      > getc(stdin);[/color]

      To eat the \n produced by the enter key
      [color=blue]
      >[/color]

      snip
      [color=blue]
      >
      >It maintains a list of words and randomly selects a word from the list when
      >the program is started. The word is printed to the screen, but all
      >characters have been replaced with a '?'. The user is asked to guess a
      >letter in the word, and he if he guesses correctly the '?'s hiding that
      >particular letter is replaced with the letter itself. This continues until
      >the entire word has been revealed.
      >
      >My problem is that the code for obtaining user input isn't very robust.
      >Say the user presses <tab>the_letter <enter>, it doesn't work anymore.
      >Whatever I type next, the character passed to handle_guess() is "messed up".[/color]

      You could use fgets to read into a three character string. Then
      confirm the second character is \n insuring the user typed exactly one
      character before pressing enter. Then use isalpha() or islower() to
      verify he typed a valid character. If any of these tests fail, tell
      him to follow the instructions more closely and loop back to repeat
      the prompt.


      <<Remove the del for email>>

      Comment

      • CBFalconer

        #4
        Re: Robustify code dealing with input

        Eric Lilja wrote:[color=blue]
        >
        > Hello, consider the following complete program:
        >[/color]
        .... snip code ...[color=blue]
        >
        > It maintains a list of words and randomly selects a word from the
        > list when the program is started. The word is printed to the
        > screen, but all characters have been replaced with a '?'. The user
        > is asked to guess a letter in the word, and he if he guesses
        > correctly the '?'s hiding that particular letter is replaced with
        > the letter itself. This continues until the entire word has been
        > revealed.
        >
        > My problem is that the code for obtaining user input isn't very
        > robust. Say the user presses <tab>the_letter <enter>, it doesn't
        > work anymore. Whatever I type next, the character passed to
        > handle_guess() is "messed up". Here's a screen dump from such a
        > session:[/color]
        .... snip ...[color=blue]
        >
        > As you can see, it remains in a broken state because of that
        > <tab>. How can improve the code for dealing with user input so
        > I can counter this problem?
        >
        > Any other comments regarding the code are welcome also.[/color]

        Here is a slightly more robust version of your code. Note the
        handling of EOF and the use of flushln. Your basic structure is
        fairly good, in that you saw off portions of the problem for
        handling by functions, and check the data. You could go further in
        this direction. I recommend the consistent use of define before
        use. This avoids unnecessary isolated prototypes and their
        maintenance.

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

        /* ---------------- */

        static void handle_guess(co nst char *actual, char *current,
        const char c)
        {
        int position = 0;
        char *posn;

        assert(strlen(a ctual) == strlen(current) );

        if (!(posn = strchr(actual, c))) {
        printf("No match for the letter %c, sorry.\n", c);
        return;
        }
        else {
        do {
        position = posn - actual;
        if (current[position] == c) {
        printf("You already guessed the letter %c\n", c);
        return;
        }
        else if (actual[position] == c)
        current[position] = c;
        } while (*(++posn));
        }
        } /* handle_guess */

        /* ---------------- */

        static int has_char(const char *str, const char c)
        {
        unsigned i;

        /* when you see strlen within a loop,
        look for ways to avoid move it to initialization */
        for (i = strlen(str); i-- > 0;)
        if (str[i] == c) return 1;
        return 0;
        } /* has_char */

        /* ---------------- */

        static void flushln(FILE *fp)
        {
        int ch;

        while (('\n' != (ch = getc(fp))) && (EOF != ch)) continue;
        } /* flushln */

        /* ---------------- */

        int main(void)
        {
        const char * words[] = {
        "hello", "usenet", "breakfast"
        };
        const char *actual;
        char *current, ch;

        srand(time(NULL ));

        actual = words[rand() % 3];
        if (!(current = malloc(1 + strlen(actual)) ))
        return EXIT_FAILURE;
        memset(current, '?', strlen(actual)) ;
        current[strlen(actual)] = '\0';

        while (has_char(curre nt, '?')) {
        printf("The word is: %s\n", current);
        printf("Type a letter: "); fflush(stdin);
        if (EOF == (ch = getc(stdin))) return 0;
        flushln(stdin);

        handle_guess(ac tual, current, tolower(ch));
        }
        printf("Congrat ulations! The word was: %s\n", current);

        free(current);
        return EXIT_SUCCESS;
        } /* main */

        --
        "If you want to post a followup via groups.google.c om, don't use
        the broken "Reply" link at the bottom of the article. Click on
        "show options" at the top of the article, then click on the
        "Reply" at the bottom of the article headers." - Keith Thompson

        Comment

        • pete

          #5
          Re: Robustify code dealing with input

          CBFalconer wrote:
          [color=blue]
          > char *current, ch;[/color]
          [color=blue]
          > if (EOF == (ch = getc(stdin))) return 0;[/color]

          I don't think that EOF is guaranteed
          to be within the range of ch.

          --
          pete

          Comment

          • Emmanuel Delahaye

            #6
            Re: Robustify code dealing with input

            Eric Lilja wrote on 04/06/05 :[color=blue]
            > static int has_char(const char *, const char);[/color]

            'char' parameters don't really exist. There are converted to int
            (probably with some extra code). Stick to simplicity:

            static int has_char(const char *, int char);

            --
            Emmanuel
            The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
            The C-library: http://www.dinkumware.com/refxc.html

            ..sig under repair

            Comment

            • Eric Lilja

              #7
              Re: Robustify code dealing with input


              "Eric Lilja" wrote:
              [My original message snipped]

              Thanks for all the replies! Now I'm sure I can improve the input code, and
              other things as well, thanks!

              / Eric


              Comment

              • Eric Sosman

                #8
                Re: Robustify code dealing with input

                Emmanuel Delahaye wrote:[color=blue]
                > Eric Lilja wrote on 04/06/05 :
                >[color=green]
                >> static int has_char(const char *, const char);[/color]
                >
                > 'char' parameters don't really exist. There are converted to int
                > (probably with some extra code). [...][/color]

                6.5.2.2/7 and /8 appear to disagree with this claim.

                --
                Eric Sosman
                esosman@acm-dot-org.invalid

                Comment

                • Paul Mesken

                  #9
                  Re: Robustify code dealing with input

                  On Sat, 04 Jun 2005 06:11:36 GMT, pete <pfiland@mindsp ring.com> wrote:
                  [color=blue]
                  >CBFalconer wrote:
                  >[color=green]
                  >> char *current, ch;[/color]
                  >[color=green]
                  >> if (EOF == (ch = getc(stdin))) return 0;[/color]
                  >
                  >I don't think that EOF is guaranteed
                  >to be within the range of ch.[/color]

                  Yes, this is a FAQ (12.1). It should be bigger than char (because EOF
                  is an "out of band" return value).

                  Comment

                  • CBFalconer

                    #10
                    Re: Robustify code dealing with input

                    pete wrote:[color=blue]
                    >
                    > CBFalconer wrote:
                    >[color=green]
                    > > char *current, ch;[/color]
                    >[color=green]
                    > > if (EOF == (ch = getc(stdin))) return 0;[/color]
                    >
                    > I don't think that EOF is guaranteed
                    > to be within the range of ch.[/color]

                    Gulp - goofed. Should be int. I added that test at the last
                    minute when an EOF left the program chasing its tail.

                    --
                    "If you want to post a followup via groups.google.c om, don't use
                    the broken "Reply" link at the bottom of the article. Click on
                    "show options" at the top of the article, then click on the
                    "Reply" at the bottom of the article headers." - Keith Thompson


                    Comment

                    • pete

                      #11
                      Re: Robustify code dealing with input

                      Eric Sosman wrote:[color=blue]
                      >
                      > Emmanuel Delahaye wrote:[color=green]
                      > > Eric Lilja wrote on 04/06/05 :
                      > >[color=darkred]
                      > >> static int has_char(const char *, const char);[/color]
                      > >
                      > > 'char' parameters don't really exist. There are converted to int
                      > > (probably with some extra code). [...][/color]
                      >
                      > 6.5.2.2/7 and /8 appear to disagree with this claim.[/color]

                      On a related topic,
                      I don't think it's usually a good idea to use small arithmetic types.
                      A common exception would be except in arrays,
                      and I'm sure there are many other exceptional cases.

                      Even though there's no promotion here:
                      static int has_char(const char *, const char);
                      the facts is that small (lower ranking than int) arithmetic types
                      do get automatically converted a lot.
                      Sometimes they can change from unsigned to signed,
                      in ways that aren't really that obvious.
                      In a situation where INT_MAX equals USHRT_MAX,
                      trying to increment an unsigned short until it rolls over
                      yields undefined behavior.

                      A single instance of small arithmetic type represents a potential
                      saving of memory,
                      but it is just as likely to be implemented on a int boundary
                      with masking operations.

                      --
                      pete

                      Comment

                      • pete

                        #12
                        Re: Robustify code dealing with input

                        CBFalconer wrote:
                        [color=blue]
                        > static void handle_guess(co nst char *actual, char *current,
                        > const char c)
                        > {
                        > int position = 0;
                        > char *posn;[/color]
                        [color=blue]
                        > position = posn - actual;[/color]

                        On June 3, 6:22 pm, you said
                        "So, in general, pointer subtraction serves no useful purpose."



                        Are you sticking with that story?

                        --
                        pete

                        Comment

                        • CBFalconer

                          #13
                          Re: Robustify code dealing with input

                          pete wrote:[color=blue]
                          > CBFalconer wrote:
                          >[color=green]
                          >> static void handle_guess(co nst char *actual, char *current,
                          >> const char c)
                          >> {
                          >> int position = 0;
                          > > char *posn;[/color]
                          >[color=green]
                          >> position = posn - actual;[/color]
                          >
                          > On June 3, 6:22 pm, you said
                          > "So, in general, pointer subtraction serves no useful purpose."
                          >
                          > Are you sticking with that story?[/color]

                          Yes. This is not "in general". This is two pointers to the same
                          object, namely the string *actual.

                          --
                          "If you want to post a followup via groups.google.c om, don't use
                          the broken "Reply" link at the bottom of the article. Click on
                          "show options" at the top of the article, then click on the
                          "Reply" at the bottom of the article headers." - Keith Thompson


                          Comment

                          • Keith Thompson

                            #14
                            Re: Robustify code dealing with input

                            CBFalconer <cbfalconer@yah oo.com> writes:[color=blue]
                            > pete wrote:[color=green]
                            >> CBFalconer wrote:
                            >>[color=darkred]
                            >>> static void handle_guess(co nst char *actual, char *current,
                            >>> const char c)
                            >>> {
                            >>> int position = 0;
                            >> > char *posn;[/color]
                            >>[color=darkred]
                            >>> position = posn - actual;[/color]
                            >>
                            >> On June 3, 6:22 pm, you said
                            >> "So, in general, pointer subtraction serves no useful purpose."
                            >>
                            >> Are you sticking with that story?[/color]
                            >
                            > Yes. This is not "in general". This is two pointers to the same
                            > object, namely the string *actual.[/color]

                            Your original statement was ambiguous. It could easily be interpreted
                            to mean that pointer subtraction is never useful. I think what you
                            meant is that pointer subtraction is not always useful.

                            --
                            Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
                            San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
                            We must do something. This is something. Therefore, we must do this.

                            Comment

                            Working...