Citesc intrările utilizatorului din stdin în simplu C. Problema este că vreau o implementare sănătoasă care să fie robustă la erori și să restricționeze utilizatorul la un anumit input și să nu fie nașpa din punct de vedere al complexității. Funcția get_strings() citește intrarea char cu char atâta timp cât nu există o linie nouă (
EOF și toate caracterele trec prin isalpha() test. Dar vreau să păstrez spațiile.
Câteva puncte care (cred că) merită o atenție specială în timpul revizuirii:
- – Mi-ar plăcea să scap de bucla while exterioară care, practic, testează dacă utilizatorul tocmai a apăsat Enter fără nicio introducere semnificativă.
- – Chiar trebuie să folosesc ferror()? fgetc returnează deja EOF atunci când ceva nu a mers bine, dar eu voi opri doar citirea din flux în loc să îi spun utilizatorului că ceva nu a mers bine.
- – Nu se întâmplă de fiecare dată, dar se întâmplă: Un
rămâne în fluxul stdin, iar data viitoare când vreau să obțin o intrare semnificativă, fgetc() este pur și simplu sărit. Nu contează aici, dar contează atunci când pun întrebări de tip Da/Nu cu un singur caracter. Nu pot scăpa de această problemă decât cu o construcție care șterge toate lucrurile anterioare care rămân în stdin. Pentru aceasta, consultați al doilea bloc de cod.
Așadar, mă ocup în mod total greșit de această problemă? Există practici mai bune pe care să le îmbrățișez? Mie mi se pare foarte greșit, iar greșit este întotdeauna rău.
/** @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 != '
') {
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
");
}
}
if (ferror(in_stream)) {
bail_out(EXIT_FAILURE, "Error while reading from stream");
}
}
if(string) {
string[len] = ' ';
} else {
printf("
Finished dictionary...
");
}
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] == ' ') 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;
}
Al doilea CodeBlock cu un caz mai simplu:
while(1) {
char tmp;
printf("Please enter your guess [a-z]: ");
guess = fgetc(stdin);
/* Jump back to start of loop */
if (guess == '
') {
continue;
}
/* HERE IS THE CLEAR FOR STDIN
This part really just eats all remaining
s from the user,
so that later inputs can start uninterrupted. Can I get rid of
it in some better way? */
while((tmp = getchar()) != '
' && tmp != EOF);
if(!isalpha(guess)) {
fprintf(stderr, "Enter a valid letter [a-z]!
");
continue;
}
}
Arhitectură
stdin este, de obicei, cu buffer de linie. Deci nu se dă nimic la fgetc() până când utilizatorul apasă Enter. Codul OP va afișa mai multe mesaje de eroare în cazul unei intrări de tipul „Hello 123”. Este mai bine să separați intrarea utilizatorului de validarea intrării. Citiți linia de intrare a utilizatorului cu fgets() sau o versiune proprie ca fgets() are unele puncte slabe. Apoi validați intrarea.
char *input;
while ((input = my_gets()) != NULL) {
if (valid_phrase(input)) {
foo(input);
} else {
fprintf(stderr, "Invalid input
");
}
free(input);
}
În ceea ce privește „Mi-ar plăcea să scap de bucla while exterioară”. Această buclă există pentru a consuma în tăcere '. Dacă doriți o buclă care să facă acest lucru, este suficient să precedați bucla interioară cu
'
int ch;
while ((ch = fgetc()) == '
')
;
ungetc(ch, stdin);
char ch
La ch nu este cel mai bun tip. fgetc() returnează de obicei 257 valori diferite [0-255] și EOF. Pentru a le distinge în mod corespunzător, salvați rezultatul într-un fișier int.
// bad
char ch;
..
while (string == NULL && ch != EOF) {
while (EOF != (ch = fgetc(in_stream)) && ch != '
') {
// better
int ch;
..
while (string == NULL && ch != EOF) {
while (EOF != (ch = fgetc(in_stream)) && ch != '
') {
Același lucru pentru char tmp;
realloc()
Cast nu este necesar.
Modificați pentru out of memory pentru a elibera string – nu este necesar dacă codul va ieși pur și simplu din memorie, dar este o bună practică pentru a vă pune jucăriile (pointerul codului) deoparte.
// 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;
O bună utilizare a lui sizeof(*strings) de mai jos. Se recomandă simplificarea.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings));
strings = realloc(strings, sizeof *strings * (index+1));
size_t len
Bună utilizare a size_t pentru a reprezenta dimensiunea unui array. În mod curios, codul nu face același lucru cu int index;. Recomandare size_t index;
is...()
Cu utilizarea int ch, nu este nevoie de cast. Deoarece acesta este un test logic, se recomandă utilizarea ! mai degrabă decât aritmetica == 0.
// if (ch != ' ' && isalpha((int)ch) == 0) {
if (ch != ' ' && !isalpha(ch)) {
Următoarea variantă poate fi mai ușor de înțeles – mai puține negații. (Problemă de stil)
if (!(ch == ' ' || isalpha(ch))) {
ferror()
Verificare plăcută a if (ferror(in_stream))
Numele variabilelor
string, strings este la fel de util ca și apelarea unui număr întreg integer. Poate phrase, dictionary în schimb.
// OK
/** @brief Contains the dictionary */
static char **strings = NULL;
// Better
// comment not truly needed
static char **dictionary = NULL;
get_strings() are un nume greșit. Sună general, dar codul limitează introducerea la litere și spațiu. Poate get_words()?
Comentarii
- Am impresia că ai postat același răspuns de două ori? Oricum, acesta este răspunsul pe care îl căutam! Eram total concentrat pe utilizarea fgetc sind fgets did’nt work in the start (Din cauza
s rogue în stdin). Acest lucru pare mult mai bun și îl voi încorpora în codul meu. Mulțumesc! > . - @Haini știi că pe lângă accept, poți să dai și upvote unui răspuns dacă ți-a plăcut atât de mult ;-)-. > .
Strategie de realocare proastă
În prezent, apelați realloc() la fiecare caracter pe care îl citești. Acest lucru duce la un timp $O(n^2)$ de citire a unui șir de caractere, deoarece de fiecare dată când apelați realloc(), este posibil să fie nevoie să copiați conținutul curent în noul buffer. Ar trebui fie să alocați doar un buffer de dimensiune MAX_DATA și apoi să utilizați realloc pentru a micșora alocarea la sfârșit, fie să treceți la o strategie de realocare în care dimensiunea realocării este mărită de fiecare dată cu un factor multiplicativ (cum ar fi 2x).
Acest lucru este valabil și pentru matricea de șiruri de caractere, unde faceți același lucru.
Indentare ciudată
Indentarea dvs. este ciudată, deoarece fișierele imbricate while este la același nivel de indentare ca și bucla exterioară while bucla exterioară.
Folosiți fgets()?
Personal, aș folosi fgets() (sau o altă funcție de bibliotecă, cum ar fi readline()) pentru a citi un șir de caractere. fgets() face cam ceea ce face bucla dvs. fără toată logica codată manual.
stdbool.h, dar nu încercați să vă creați propriile bool-uri. > Por SirPython.