Ik lees gebruikersinvoer van stdin in gewone C. Het probleem is dat ik een verstandige implementatie wil die bestand is tegen fouten en de gebruiker beperkt tot een bepaalde invoer en niet slecht is in termen van complexiteit. De functie get_strings() leest invoer char door char zolang er geen nieuwe regel is (\n), geen EOF en alle karakters de isalpha() test. Maar ik wil spaties behouden.
Enkele punten die (denk ik) speciale aandacht verdienen tijdens de beoordeling:
- – Ik zou graag de buitenste while-lus die in feite test of de gebruiker zojuist op Enter heeft gedrukt zonder enige zinvolle invoer.
- – Moet ik zelfs ferror () gebruiken? fgetc retourneert al EOF als er iets mis is gegaan, maar ik zal gewoon stop met lezen uit de stream in plaats van de gebruiker te vertellen dat er iets mis is gegaan.
- – Het gebeurt niet elke keer, maar het gebeurt: A \ n blijft in de standaard stream en de volgende keer wil ik zinvolle input krijgen fgetc () wordt gewoon overgeslagen. Het maakt hier niet uit, maar het is wel als ik Ja / Nee-vragen met één teken stel. Ik kan dit probleem alleen oplossen met een constructie die alle voorgaande dingen opruimt die in standaard zijn achtergelaten. Zie hiervoor het tweede codeblok .
Dus, pak ik dit helemaal verkeerd aan? Zijn er betere praktijken om te omarmen? Het ziet er erg onhandig uit, en onhandig is altijd slecht.
/** @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; } Tweede codeblok met een eenvoudiger hoofdlettergebruik:
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; } } Opmerkingen
Answer
Architectuur
stdin is meestal lijn gebufferd . Er wordt dus niets gegeven aan fgetc() totdat de gebruiker op Enter drukt. OP-code geeft meerdere foutmeldingen met invoer als “Hallo 123”. Het is beter om gebruikersinvoer te scheiden van invoervalidatie. Lees de regel met gebruikersinvoer met fgets() of een eigen versie, aangezien fgets() enkele zwakke punten heeft. Vervolgens valideer de invoer.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Betreffende “Ik zou graag de buitenste while-lus verwijderen”. Die lus bestaat om stilletjes "\n" te consumeren. Als u dat wilt dat een lus dat doet, laat u de binnenste lus gewoon voorafgaan met
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
De ch is niet het beste type. fgetc() retourneert doorgaans 257 verschillende waarden [0-255] en EOF. Om ze goed te onderscheiden, slaat u het resultaat op in een 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") { Hetzelfde voor char tmp;
realloc()
Cast niet nodig.
Aanpassen voor onvoldoende geheugen om string vrij te maken – niet nodig als de code gewoon weggaat, maar een goede gewoonte om je speelgoed (code “s pointer) weg.
// 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; Goed gebruik van sizeof(*strings) hieronder. Vereenvoudiging aanbevelen.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Goed gebruik van size_t om de grootte van een array weer te geven. Vreemd genoeg doet code niet hetzelfde met int index;. Beveel size_t index;
is...()
Met het gebruik van int ch is casten niet nodig. is een logische test, raad aan om ! te gebruiken in plaats van rekenkundig == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Het volgende is misschien gemakkelijker te begrijpen – minder ontkenningen. (Stijlprobleem)
if (!(ch == " " || isalpha(ch))) { ferror()
Leuke controle van if (ferror(in_stream))
Variabelenamen
string, strings is net zo nuttig als een geheel getal integer aanroepen. Misschien phrase, dictionary.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() heeft een verkeerde naam. Het klinkt algemeen, maar code beperkt de invoer tot letters en spaties.Misschien get_words()?
Reacties
- Ik heb het gevoel dat je hetzelfde antwoord twee keer hebt gepost? Hoe dan ook, dit is het antwoord dat ik zocht! Ik was helemaal gefocust op het gebruik van fgetc sind fgets deed ‘ nt werk in het begin (vanwege rogue \ ns in stdin). Dit lijkt veel beter en ik zal dit in mijn code opnemen. Bedankt!
- @Haini je weet dat je niet alleen kunt accepteren, maar ook een antwoord kunt upvoten als je het zo leuk vond 😉
Antwoord
Slechte herverdelingsstrategie
Momenteel roep je realloc() aan bij elk teken dat je leest. Dit resulteert in een \ $ O (n ^ 2) \ $ tijd om een string te lezen, omdat elke keer dat je realloc() aanroept, het mogelijk de huidige inhoud naar de nieuwe buffer moet kopiëren . U moet ofwel gewoon een buffer met de grootte MAX_DATA toewijzen en vervolgens realloc gebruiken om de toewijzing aan het einde te verkleinen, of u moet overstappen op een herverdelingsstrategie waarbij de grootte van de herverdeling elke keer met een vermenigvuldigingsfactor wordt vergroot (zoals 2x).
Dit geldt ook voor uw reeks strings, waar u hetzelfde doet.
Vreemde inspringing
Uw inspringing is vreemd omdat uw geneste while lus zich op hetzelfde inspringingsniveau bevindt als de buitenste while lus.
Gebruik fgets ()?
Ik zou persoonlijk fgets() gebruiken (of een andere bibliotheekfunctie zoals readline()) om een string te lezen. fgets() doet vrijwel wat je lus doet zonder alle handgecodeerde logica.
stdbool.h, maar gebruik ‘ probeer je eigen bools te rollen.