Leí la entrada del usuario de stdin en C. El problema es que quiero una implementación sensata que sea robusta a los errores y restrinja al usuario a una determinada entrada y no apesta en términos de complejidad. La función get_strings() lee el carácter de entrada por char siempre que no haya una línea nueva (\n), no EOF y todos los caracteres pasen el isalpha() prueba. Pero quiero mantener los espacios.
Algunos puntos que (creo) merecen especial atención durante la revisión:
- – Me encantaría deshacerme del bucle while externo que básicamente está probando si el usuario acaba de presionar Enter sin ninguna entrada significativa.
- – ¿Necesito usar ferror ()? fgetc ya devuelve EOF cuando algo salió mal, pero lo haré dejar de leer de la secuencia en lugar de decirle al usuario que algo salió mal.
- – No sucede siempre, pero sucede: A \ n permanece en la secuencia stdin y la próxima vez quiero obtener una entrada significativa fgetc () simplemente se omite. No importa aquí, pero lo hace cuando hago preguntas de un solo carácter Sí / No. Solo puedo deshacerme de este problema con una construcción que borre todas las cosas anteriores que quedan en stdin. Vea el segundo bloque de código para esto .
Entonces, ¿estoy manejando esto totalmente mal? ¿Hay mejores prácticas que adoptar? Me parece realmente torpe, y torpe siempre es malo.
/** @brief Contains the dictionary */ static char **strings = NULL; /** @brief Helps with containing the dicionary */ static char *string; /* Reads input char by char with fgetc() */ static char *get_strings() { char *string = NULL; char ch; size_t len = 0; while (string == NULL && ch != EOF) { while (EOF != (ch = fgetc(in_stream)) && ch != "\n") { if (ch != " " && isalpha((int)ch) == 0) { fprintf(stderr, "Only [a-z] is a valid input. | \t" "| Input another or end with CTRL+D: "); continue; } string = (char*) realloc(string, len+2); if (string == NULL) { bail_out(EXIT_FAILURE, "realloc(3) failed"); } string[len++] = toupper(ch); if (len >= MAX_DATA) { bail_out(EXIT_FAILURE, "Input too long\n"); } } if (ferror(in_stream)) { bail_out(EXIT_FAILURE, "Error while reading from stream"); } } if(string) { string[len] = "\0"; } else { printf("\nFinished dictionary...\n"); } printf("Added string: %s | Input another or end with CTRL+D: ", string); return string; } /* Saves the returned strings from get_strings() in a linked list */ static void read_dict() { int index; for (index = 0; (string = get_strings()); ++index) { if (string[0] == "\0") continue; strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); if (strings == NULL) { bail_out(EXIT_FAILURE, "realloc(3) failed"); } strings[index] = string; } /* Take a note of how many entries we have yet. */ dict_size = index; } Segundo CodeBlock con un caso más simple:
while(1) { char tmp; printf("Please enter your guess [a-z]: "); guess = fgetc(stdin); /* Jump back to start of loop */ if (guess == "\n") { continue; } /* HERE IS THE CLEAR FOR STDIN This part really just eats all remaining \ns from the user, so that later inputs can start uninterrupted. Can I get rid of it in some better way? */ while((tmp = getchar()) != "\n" && tmp != EOF); if(!isalpha(guess)) { fprintf(stderr, "Enter a valid letter [a-z]!\n"); continue; } } Comentarios
Responder
Arquitectura
stdin suele tener línea almacenada en búfer . Así que no se le da nada a fgetc() hasta que el usuario presione Enter . El código OP dará varios mensajes de error con entradas como «Hola 123». Es mejor separar la entrada del usuario de la validación de entrada. Lea la línea de entrada del usuario con fgets() o alguna versión propia, ya que fgets() tiene algunas debilidades. Luego valide la entrada.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Con respecto a «Me encantaría deshacerme del ciclo while externo». Ese bucle existe para consumir "\n" silenciosamente. Si desea que un bucle haga eso, simplemente precedido del bucle interno con
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
El ch no es el mejor tipo. fgetc() devuelve normalmente 257 valores diferentes [0-255] y EOF. Para distinguirlos correctamente, guarde el resultado en un int.
// bad char ch; .. while (string == NULL && ch != EOF) { while (EOF != (ch = fgetc(in_stream)) && ch != "\n") { // better int ch; .. while (string == NULL && ch != EOF) { while (EOF != (ch = fgetc(in_stream)) && ch != "\n") { Lo mismo para char tmp;
realloc()
Cast no es necesario.
Modificar por falta de memoria para liberar string; no es necesario si el código simplemente sale, pero una buena práctica para colocar sus juguetes (código «s) de distancia.
// string = (char*) realloc(string, len+2); char * new_string = realloc(string, len+2); if (new_string == NULL) { free(string); bail_out(EXIT_FAILURE, "Out of memory"); } string = new_string; Buen uso de sizeof(*strings) a continuación. Se recomienda simplificar.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Buen uso de size_t para representar un tamaño de matriz. Curiosamente, el código no hace lo mismo con int index;. Recomendar size_t index;
is...()
Con int ch, no es necesario emitir. Esta es una prueba lógica, recomendamos usar ! en lugar de == 0 aritmética.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Lo siguiente puede ser más fácil de entender: menos negaciones. (Problema de estilo)
if (!(ch == " " || isalpha(ch))) { ferror()
Buen control de if (ferror(in_stream))
Nombres de variables
string, strings es tan útil como llamando a un entero integer. Tal vez phrase, dictionary en su lugar.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() tiene un nombre incorrecto. Suena general, pero el código restringe la entrada a letras y espacios.¿Quizás get_words()?
Comentarios
- Siento que publicaste la misma respuesta dos veces. De todos modos, ¡esta es la respuesta que estaba buscando! Estaba totalmente concentrado en usar fgetc sind fgets ‘ no funcionó al principio (debido a errores \ ns en stdin). Esto parece mucho mejor y lo incorporaré en mi código. ¡Gracias!
- @Haini sabes que además de aceptar, también puedes votar a favor una respuesta si tanto te gustó 😉
Responder
Mala estrategia de reasignación
Actualmente, llamas realloc() en cada carácter que lees. Esto da como resultado un \ $ O (n ^ 2) \ $ tiempo para leer una cadena, porque cada vez que llama a realloc(), es posible que deba copiar el contenido actual en el nuevo búfer . Debe asignar un búfer de tamaño MAX_DATA y luego usar realloc para reducir la asignación al final, o cambiar a una estrategia de reasignación donde el tamaño de la reasignación se incrementa en un factor multiplicativo cada vez (como 2x).
Esto también se aplica a su matriz de cadenas, donde hace lo mismo.
Sangría extraña
Su sangría es extraña porque su ciclo while anidado está en el mismo nivel de sangría que el ciclo externo while.
¿Usar fgets ()?
Yo personalmente usaría fgets() (o alguna otra función de biblioteca como readline()) para leer una cadena. fgets() hace prácticamente lo que hace su bucle sin toda la lógica codificada a mano.
stdbool.h, pero no ‘ No intente hacer sus propios bools.