Ho letto linput dellutente da stdin in chiaro C. Il problema è che voglio unimplementazione sana che sia robusta agli errori e limiti lutente a un certo input e non faccia schifo in termini di complessità. La funzione get_strings() legge input char da finché non cè una nuova riga (\n), nessun EOF e tutti i caratteri passano il isalpha() test. Ma voglio mantenere gli spazi.
Alcuni punti che (penso) meritano unattenzione speciale durante la revisione:
- – Mi piacerebbe sbarazzarmi del ciclo while esterno che fondamentalmente sta verificando se lutente ha appena premuto Invio senza alcun input significativo.
- – Devo anche usare ferror ()? fgetc restituisce già EOF quando qualcosa è andato storto, ma lo farò smetti di leggere dallo stream invece di dire allutente che qualcosa è andato storto.
- – Non succede ogni volta ma succede: A \ n rimane nello stream stdin e la prossima volta che voglio ottenere un input significativo fgetc () viene semplicemente saltato. Non importa qui, ma lo è quando pongo domande Sì / No a carattere singolo. Posso eliminare questo problema solo con un costrutto che cancella tutte le cose precedenti che sono rimaste in stdin. Vedi il secondo blocco di codice per questo .
Quindi, sto gestendo tutto in modo sbagliato? Esistono pratiche migliori da adottare? Mi sembra davvero goffo, e goffo è sempre un male.
/** @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; } Secondo CodeBlock con un caso più semplice:
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; } } Commenti
Rispondi
Architettura
stdin di solito è buffer di riga . Quindi non viene dato nulla a fgetc() finché lutente non preme Invio . Il codice OP darà più messaggi di errore con input come “Hello 123”. Meglio separare linput dellutente dalla convalida dellinput. Leggi la riga di input dellutente con fgets() o qualche tua versione poiché fgets() presenta alcuni punti deboli. Quindi convalida linput.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Riguardo a “Mi piacerebbe sbarazzarmi del ciclo while esterno”. Quel ciclo esiste per consumare silenziosamente "\n". Se desideri che un ciclo lo faccia, fai precedere il ciclo interno con
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
ch non è il tipo migliore. fgetc() restituisce in genere 257 valori diversi [0-255] e EOF. Per distinguerli correttamente, salva il risultato in 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 stesso per char tmp;
realloc()
Trasmissione non necessaria.
Modifica per memoria esaurita per liberare string – non necessaria se il codice esce semplicemente, tuttavia è buona norma inserire i tuoi giocattoli (codice “s pointer).
// 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; Buon uso di sizeof(*strings) di seguito. Consiglia la semplificazione.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Buon uso di size_t per rappresentare la dimensione di un array. Curiosamente il codice non fa lo stesso con int index;. Consiglia size_t index;
is...()
Utilizzando int ch, non è necessario il cast. Poiché è un test logico, consiglia di utilizzare ! anziché aritmetica == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Quanto segue potrebbe essere più facile da capire: meno negazioni. (Problema di stile)
if (!(ch == " " || isalpha(ch))) { ferror()
Bel assegno di if (ferror(in_stream))
I nomi delle variabili
string, strings è utile quanto chiamando un numero intero integer. Forse phrase, dictionary invece.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() ha un nome errato. Sembra generale, ma il codice limita linput a lettere e spazi.Forse get_words()?
Commenti
- Mi sembra che tu abbia pubblicato la stessa risposta due volte? Comunque, questa è la risposta che stavo cercando! Ero totalmente concentrato sulluso di fgetc sind fgets ‘ non funzionava allinizio (a causa di rogue \ ns in stdin). Questo sembra molto meglio e lo incorporerò nel mio codice. Grazie!
- @Haini sai che oltre ad accettare, puoi anche votare una risposta se ti è piaciuta così tanto 😉
Risposta
Strategia di realloc non valida
Attualmente chiami realloc() su ogni carattere che leggi. Ciò si traduce in \ $ O (n ^ 2) \ $ tempo per leggere una stringa, perché ogni volta che chiami realloc(), potrebbe essere necessario copiare il contenuto corrente nel nuovo buffer . Dovresti semplicemente allocare un buffer di dimensione MAX_DATA e quindi utilizzare realloc per ridurre lallocazione alla fine o passare a una strategia di riallocazione dove la dimensione di riallocazione viene aumentata ogni volta di un fattore moltiplicativo (come 2x).
Questo vale anche per il tuo array di stringhe, dove fai la stessa cosa.
Strano rientro
Il tuo rientro è strano perché il tuo ciclo while nidificato si trova allo stesso livello di rientro del ciclo while esterno.
Usa fgets ()?
Io personalmente userei fgets() (o qualche altra funzione di libreria come readline()) per leggere una stringa. fgets() fa praticamente quello che fa il tuo ciclo senza tutta la logica codificata a mano.
stdbool.h, ma non ‘ t provare a lanciare i tuoi bool.