Jeg leste brukerinngang fra stdin i vanlig C. Problemet er at jeg vil ha en sunn implementering som er robust for feil og begrenser brukeren til en viss inngang og ikke suger når det gjelder kompleksitet. Funksjonen get_strings() leser inngangstegn etter char så lenge det ikke er noen ny linje (\n), ingen EOF og alle tegnene passerer isalpha() test. Men jeg vil beholde mellomrom.
Noen punkter som (jeg tror) fortjener spesiell oppmerksomhet under gjennomgangen:
- – Jeg vil gjerne bli kvitt ytre mens sløyfe som i utgangspunktet tester hvorvidt brukeren bare trykket på Enter uten noen meningsfylt inngang.
- – Må jeg til og med bruke ferror ()? fgetc returnerer allerede EOF når noe gikk galt, men jeg vil bare slutte å lese fra strømmen i stedet for å fortelle brukeren at noe gikk galt.
- – Det skjer ikke hver gang, men det skjer: A \ n forblir i stdin-strømmen og neste gang vil jeg få meningsfulle innspill fgetc () blir bare hoppet over. Det spiller ingen rolle her, men det gjør det når jeg stiller enkeltkarakter Ja / Nei. Jeg kan bare kvitte meg med dette problemet med en konstruksjon som fjerner alle tidligere ting som er igjen i stdin. Se den andre kodeblokken for dette .
Så, håndterer jeg dette helt galt? Er det bedre fremgangsmåter å omfavne? Det ser veldig klumpete ut for meg, og klumpete er alltid dårlig.
/** @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; } Second CodeBlock med en mer enkel sak:
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; } } Kommentarer
Svar
Arkitektur
stdin er vanligvis linjebufret . Så ingenting er gitt til fgetc() før brukeren treffer Enter . OP-kode vil gi flere feilmeldinger med inndata som «Hello 123». Bedre å skille brukerinngang fra validering av inndata. Les linjen med brukerinngang med fgets() eller en egen versjon, da fgets() har noen svakheter. Så valider innspillet.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Når det gjelder «Jeg vil gjerne bli kvitt den ytre mens sløyfen». Den sløyfen eksisterer for å tømme "\n". Hvis du vil at en løkke skal gjøre det, gikk du bare foran den indre sløyfen med
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
ch er ikke den beste typen. fgetc() returnerer typisk 257 forskjellige verdier [0-255] og EOF. For å skille dem ordentlig, lagre resultatet i en 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") { Samme for char tmp;
realloc()
Cast trenger ikke.
Endre for lite minne for å frigjøre string – ikke nødvendig hvis koden rett og slett kommer ut, men god praksis å sette lekene dine (kode «s peker) borte.
// 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; God bruk av sizeof(*strings) nedenfor. Anbefaler forenkling.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
God bruk av size_t for å representere en matrisestørrelse. Merkelig nok gjør ikke kode det samme med int index;. Anbefal size_t index;
is...()
Når du bruker int ch, er det ikke behov for rollebesetning. Siden is er en logisk test, anbefaler å bruke ! i stedet for aritmetikk == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Følgende kan være lettere å forstå – færre negasjoner. (Stilutgave)
if (!(ch == " " || isalpha(ch))) { ferror()
Fin sjekk av if (ferror(in_stream))
Variabelnavn
string, strings er like nyttig som kaller et heltall integer. Kanskje phrase, dictionary i stedet.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() heter feil. Det høres generelt ut, men kode begrenser inngang til bokstaver og mellomrom.Kanskje get_words()?
Kommentarer
- Jeg føler at du la ut det samme svaret to ganger? Uansett, dette er svaret jeg lette etter! Jeg var helt fokusert på å bruke fgetc sind fgets fungerte ‘ nt i starten (på grunn av rogue \ ns i stdin). Dette virker mye bedre, og jeg vil innlemme dette i koden min. Takk!
- @Haini du vet at i tillegg til å godta, kan du også stemme opp et svar hvis du likte det så godt 😉
Svar
Dårlig realloc-strategi
For øyeblikket ringer du til realloc() på hvert tegn du leser. Dette resulterer i en \ $ O (n ^ 2) \ $ tid til å lese en streng, fordi det hver gang du ringer til realloc(), kan det hende at du må kopiere det gjeldende innholdet til den nye bufferen . Du skal enten bare tildele en buffer med størrelse MAX_DATA og deretter bruke realloc for å krympe tildelingen på slutten, eller endre til en omfordelingsstrategi der omdisponeringsstørrelsen økes med en multiplikasjonsfaktor hver gang (for eksempel 2x).
Dette gjelder også strengen din, der du gjør det samme.
Merkelig fordypning
Innrykkingen din er rart fordi den nestede while -sløyfen er på samme innrykknivå som den ytre while -sløyfen.
Bruk fgets ()?
Jeg vil personlig bruke fgets() (eller noen annen biblioteksfunksjon som readline()) for å lese en streng. fgets() gjør stort sett det du gjør uten all håndkodet logikk.
stdbool.h, men ikke ‘ Ikke prøv å rulle dine egne bools.