Robustify code dealing with input

E

Eric Lilja

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(const 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(actual) + 1, sizeof(char));
assert(current);
memset(current, '?', strlen(actual));

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

if(!has_char(current, '?'))
break;

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

printf("Type a letter: ");

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

handle_guess(actual, current, tolower(c));
}

printf("Congratulations! 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 == c)
return 1;

return 0;
}

static int
find_position(const 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 == c)
return i;

return -1;
}

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

assert(strlen(actual) == strlen(current));

if((position = find_position(actual, 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(actual, 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
 
P

pete

Eric said:
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(const 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(actual) + 1, sizeof(char));
assert(current);
memset(current, '?', strlen(actual));

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

if(!has_char(current, '?'))
break;

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

printf("Type a letter: ");

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

handle_guess(actual, current, tolower(c));
}

printf("Congratulations! 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 == c)
return 1;

return 0;
}

static int
find_position(const 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 == c)
return i;

return -1;
}

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

assert(strlen(actual) == strlen(current));

if((position = find_position(actual, 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(actual, 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
Click to expand...


/* BEGIN new.c */

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

static void
handle_guess(const 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(actual) + 1, sizeof(char));
if (current == NULL) {
exit(EXIT_FAILURE);
}
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(actual, current, tolower(c));
}
printf("Congratulations! The word was: %s\n", current);
free(current);
return EXIT_SUCCESS;
}

static void
handle_guess(const 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 */
 
B

Barry Schwarz

On Sat, 4 Jun 2005 04:09:37 +0200, "Eric Lilja"


snip
printf("Type a letter: ");

c = getc(stdin);
Click to expand...

Hopefully a letter.
getc(stdin);
Click to expand...

To eat the \n produced by the enter key
snip


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".
Click to expand...

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>>
 
C

CBFalconer

Eric said:
Hello, consider the following complete program:
.... snip code ...

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: .... snip ...

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.
Click to expand...

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(const char *actual, char *current,
const char c)
{
int position = 0;
char *posn;

assert(strlen(actual) == 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 == 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(current, '?')) {
printf("The word is: %s\n", current);
printf("Type a letter: "); fflush(stdin);
if (EOF == (ch = getc(stdin))) return 0;
flushln(stdin);

handle_guess(actual, current, tolower(ch));
}
printf("Congratulations! The word was: %s\n", current);

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

Eric Lilja

:
[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
 
E

Eric Sosman

Emmanuel said:
Eric Lilja wrote on 04/06/05 :
static int has_char(const char *, const char);
Click to expand...

'char' parameters don't really exist. There are converted to int
(probably with some extra code). [...]
Click to expand...

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

Paul Mesken

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

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

CBFalconer

pete said:
I don't think that EOF is guaranteed
to be within the range of ch.
Click to expand...

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

pete

Eric said:
Emmanuel said:
Eric Lilja wrote on 04/06/05 :
static int has_char(const char *, const char);
Click to expand...

'char' parameters don't really exist. There are converted to int
(probably with some extra code). [...]
Click to expand...

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

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.
 
C

CBFalconer

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

Are you sticking with that story?
Click to expand...

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

Keith Thompson

CBFalconer said:
Yes. This is not "in general". This is two pointers to the same
object, namely the string *actual.
Click to expand...

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.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

SENTINEL CONTROL LOOP WHEN DEALING WITH TWO ARRAYS 1
C language. work with text 3
STRING - Remove small letters from string 1
If(strcmp(str, "") == 0) - What does this line of code mean? 0
A process take input from /proc/<pid>/fd/0, but won't process it 0
How do i Do this function(dealing with arrays) 1
How can I view / open / render / display a pdf file with c code? 0
Problem with displaying character that code number is 219 (after SetConsoleTextAttribute)? 3

Members online

Total: 24 (members: 1, guests: 23)
Robots: 220

Forum statistics

Threads
473,764
Messages
2,569,566
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top