Ich lese Benutzereingaben von stdin in der Ebene C. Das Problem ist, dass ich eine vernünftige Implementierung möchte, die fehlerresistent ist und den Benutzer auf eine bestimmte Eingabe beschränkt und die Komplexität nicht beeinträchtigt. Die Funktion get_strings() liest Eingabezeichen durch char solange es keine neue Zeile gibt (\n), keine EOF und alle Zeichen übergeben die isalpha() test. Aber ich möchte Leerzeichen behalten.
Einige Punkte, die (glaube ich) während der Überprüfung besondere Aufmerksamkeit verdienen:
- – Ich würde das gerne loswerden äußere while-Schleife, die im Grunde testet, ob der Benutzer gerade die Eingabetaste ohne sinnvolle Eingabe gedrückt hat.
- – Muss ich überhaupt ferror () verwenden? fgetc gibt EOF bereits zurück, wenn etwas schief gelaufen ist, aber ich werde es einfach tun Hören Sie auf, aus dem Stream zu lesen, anstatt dem Benutzer mitzuteilen, dass ein Fehler aufgetreten ist.
- – Es passiert nicht jedes Mal, aber es passiert: A \ n bleibt im Standard-Stream und das nächste Mal möchte ich aussagekräftige Eingaben erhalten fgetc () wird einfach übersprungen. Es spielt hier keine Rolle, aber es tut es, wenn ich einzelne Zeichen mit Ja / Nein-Fragen stelle. Ich kann dieses Problem nur mit einem Konstrukt beseitigen, das alle vorherigen Dinge löscht, die in stdin übrig bleiben. Siehe hierzu den zweiten Codeblock
Also, gehe ich damit völlig falsch um? Gibt es bessere Praktiken, um sie zu akzeptieren? Es sieht für mich wirklich klobig aus und klobig ist immer schlecht.
/** @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; } Zweiter CodeBlock mit einem einfacheren Fall:
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; } } Kommentare
- Schnell Hinweis: Verwenden Sie lieber Boolens aus < stdbool.h > oder definieren Sie anstelle von 1 und 0. Es ‚ ist klarer, was du meinst
- @Zorgatone Ich stimme dir halb zu; benutze immer
stdbool.h, aber nicht ‚ Versuchen Sie nicht, Ihre eigenen Bools zu rollen.
Antwort
Architektur
stdin ist normalerweise zeilengepuffert . fgetc() wird also nichts gegeben, bis der Benutzer Enter drückt. Der OP-Code gibt mehrere Fehlermeldungen mit Eingaben wie „Hallo 123“ aus. Es ist besser, Benutzereingaben von der Eingabevalidierung zu trennen. Lesen Sie die Zeile der Benutzereingaben mit fgets() oder einer eigenen Version, da fgets() einige Schwächen aufweist. Dann validieren Sie die Eingabe.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } In Bezug auf „Ich würde gerne die äußere while-Schleife loswerden“. Diese Schleife ist vorhanden, um "\n" stillschweigend zu verbrauchen. Wenn Sie möchten, dass eine Schleife dies tut, stellen Sie der inneren Schleife vor
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
Die ch ist nicht der beste Typ. fgetc() gibt normalerweise 257 verschiedene Werte zurück. [0-255] und EOF. Um sie richtig zu unterscheiden, speichern Sie das Ergebnis in einem 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") { Gleiches gilt für char tmp;
realloc()
Cast nicht erforderlich.
Ändern Sie den Speicherplatz, um string freizugeben. Wird nicht benötigt, wenn der Code einfach beendet wird. Es empfiehlt sich jedoch, Ihr Spielzeug zu platzieren (Code) „s Zeiger) entfernt.
// 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; Gute Verwendung von sizeof(*strings) unten. Vereinfachung empfehlen.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Gute Verwendung von size_t zur Darstellung einer Arraygröße. Seltsamerweise funktioniert Code nicht mit int index;. Empfehlen Sie size_t index;
is...()
Bei Verwendung von int ch ist keine Besetzung erforderlich Dies ist ein logischer Test. Empfehlen Sie die Verwendung von ! anstelle der arithmetischen == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Folgendes ist möglicherweise leichter zu verstehen – weniger Negationen. (Stilproblem)
if (!(ch == " " || isalpha(ch))) { ferror()
Schöne Überprüfung von if (ferror(in_stream))
Variablennamen
string, strings ist ebenso nützlich wie Aufruf einer Ganzzahl integer. Vielleicht phrase, dictionary stattdessen.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() ist falsch benannt. Es klingt allgemein, aber Code beschränkt die Eingabe auf Buchstaben und Leerzeichen.Vielleicht get_words()?
Kommentare
- Ich habe das Gefühl, Sie haben dieselbe Antwort zweimal gepostet? Wie auch immer, das ist die Antwort, nach der ich gesucht habe! Ich habe mich voll und ganz auf die Verwendung von fgetc konzentriert. Sind fgets am Anfang nicht ‚ (Wegen Rogue \ ns in stdin). Dies scheint viel besser zu sein und ich werde dies in meinen Code aufnehmen. Danke!
- @Haini Sie wissen, dass Sie zusätzlich zu akzeptieren auch eine Antwort positiv bewerten können, wenn es Ihnen so gut gefallen hat 😉
Antwort
Schlechte Realloc-Strategie
Derzeit rufen Sie bei jedem gelesenen Zeichen realloc() auf. Dies führt zu einer \ $ O (n ^ 2) \ $ Zeit zum Lesen einer Zeichenfolge, da bei jedem Aufruf von realloc() möglicherweise der aktuelle Inhalt in den neuen Puffer kopiert werden muss . Sie sollten entweder nur einen Puffer der Größe MAX_DATA zuweisen und dann realloc verwenden, um die Zuordnung am Ende zu verkleinern, oder zu einer Neuzuweisungsstrategie wechseln Dabei wird die Neuzuweisungsgröße jedes Mal um einen multiplikativen Faktor erhöht (z. B. 2x).
Dies gilt auch für Ihr Array von Zeichenfolgen, bei denen Sie dasselbe tun.
Seltsame Einrückung
Ihre Einrückung ist seltsam, weil sich Ihre verschachtelte while -Schleife auf derselben Einrückungsstufe befindet wie die äußere while -Schleife.
Verwenden Sie fgets ()?
Ich würde persönlich fgets() (oder eine andere Bibliotheksfunktion wie ), um eine Zeichenfolge zu lesen. fgets() macht so ziemlich das, was Ihre Schleife ohne die gesamte handcodierte Logik macht.