Jeg læste brugerinput fra stdin i almindelig C. Problemet er, at jeg ønsker en fornuftig implementering, der er robust over for fejl og begrænser brugeren til et bestemt input og ikke suger med hensyn til kompleksitet. Funktionen get_strings() læser input char ved char så længe der ikke er nogen ny linje (\n), ingen EOF og alle tegn passerer isalpha() test. Men jeg vil beholde mellemrum.
Nogle punkter, som (jeg synes) fortjener særlig opmærksomhed under gennemgangen:
- – Jeg vil meget gerne slippe af med ydre mens sløjfe, som grundlæggende tester, om brugeren bare trykkede på Enter uden nogen meningsfuld input.
- – Behøver jeg endda at bruge ferror ()? fgetc returnerer allerede EOF, når noget gik galt, men jeg vil bare stop med at læse fra strømmen i stedet for at fortælle brugeren, at der gik noget galt.
- – Det sker ikke hver gang, men det sker: A \ n forbliver i stdin-strømmen, og næste gang vil jeg få meningsfuld input fgetc () springes bare over. Det betyder ikke noget her, men det gør det, når jeg spørger enkelt karakter Ja / Nej. Jeg kan kun slippe af med dette problem med en konstruktion, der rydder alle tidligere ting, der er tilbage i stdin. Se den anden kodeblok for dette .
Så håndterer jeg dette helt forkert? Er der bedre fremgangsmåder at omfavne? Det ser virkelig klodset ud for mig, og klodset er altid dårligt.
/** @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; } Anden CodeBlock med en mere enkel sag:
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 normalt linjebufferet . Så der gives intet til fgetc(), før brugeren rammer Enter . OP-kode vil give flere fejlmeddelelser med input som “Hej 123”. Bedre at adskille brugerinput fra inputvalidering. Læs brugerlinjelinjen med fgets() eller en eller anden version af dig selv, da fgets() har nogle svagheder. Derefter valider indgangen.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Vedrørende “Jeg ville elske at slippe af med den ydre mens sløjfe”. Denne sløjfe eksisterer for lydløst at forbruge "\n". Hvis du vil have en løkke til at gøre det, skal du lige gå forud for den indvendige sløjfe med
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
ch er ikke den bedste type. fgetc() returnerer typisk 257 forskellige værdier [0-255] og EOF. For at skelne dem korrekt skal du gemme 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 ikke nødvendigt.
Rediger for ud af hukommelse for at frigøre string – ikke nødvendigt, hvis koden simpelthen kommer ud, men alligevel god praksis at sætte dit legetøj (kode “s markør) væk.
// 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 brug af sizeof(*strings) nedenfor. Anbefal forenkling.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
God brug af size_t til at repræsentere en matrixstørrelse. Nysgerrig kode gør ikke det samme med int index;. Anbefal size_t index;
is...()
Når du bruger int ch, er der ikke behov for rollebesætning. Siden is er en logisk test, anbefales at bruge ! i stedet for aritmetik == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Det følgende kan være lettere at forstå – færre negationer. (Stilproblem)
if (!(ch == " " || isalpha(ch))) { ferror()
God kontrol af if (ferror(in_stream))
Variabelnavne
string, strings er lige så nyttigt som kalder et heltal integer. Måske phrase, dictionary i stedet.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() er forkert navngivet. Det lyder generelt, men alligevel kode begrænser input til bogstaver og mellemrum.Måske get_words()?
Kommentarer
- Jeg har lyst til, at du har sendt det samme svar to gange? Anyways, dette er svaret, jeg ledte efter! Jeg var helt fokuseret på at bruge fgetc sind fgets fungerede ‘ nt i starten (på grund af rogue \ ns i stdin). Dette virker meget bedre, og jeg vil indarbejde dette i min kode. Tak!
- @Haini du ved, at du ud over at acceptere, også kan opstemme et svar, hvis du kunne lide det så meget 😉
Svar
Dårlig realloc-strategi
I øjeblikket ringer du til realloc() på hvert tegn, du læser. Dette resulterer i \ $ O (n ^ 2) \ $ tid til at læse en streng, for hver gang du ringer til realloc(), er det muligvis nødvendigt at kopiere det aktuelle indhold til den nye buffer . Du skal enten bare tildele en buffer i størrelse MAX_DATA og derefter bruge realloc til at formindske tildelingen i slutningen eller skifte til en omfordelingsstrategi hvor genallokeringsstørrelsen øges med en multiplikativ faktor hver gang (f.eks. 2x).
Dette gælder også for dit array af strenge, hvor du gør det samme.
Underlig indrykning
Din indrykning er mærkelig, fordi din indlejrede while loop er på samme indrykningsniveau som den ydre while loop.
Brug fgets ()?
Jeg bruger personligt fgets() (eller en anden biblioteksfunktion såsom readline()) for at læse en streng. fgets() gør stort set hvad din sløjfe gør uden al håndkodet logik.
stdbool.h, men don ‘ prøv ikke at rulle dine egne bools.