Czytam dane wejściowe użytkownika z stdin w zwykłym C. Problem polega na tym, że chcę rozsądnej implementacji, która jest odporna na błędy i ogranicza użytkownika do określonych danych wejściowych i nie jest do niczego pod względem złożoności. Funkcja get_strings() odczytuje znak wejściowy przez znak, o ile nie ma nowej linii (\n), nie ma EOF i wszystkie znaki przechodzą przez isalpha(). Ale chcę zachować spacje.
Niektóre punkty, które (moim zdaniem) zasługują na szczególną uwagę podczas przeglądu:
- – Chciałbym pozbyć się zewnętrzna pętla while, która w zasadzie testuje, czy użytkownik właśnie nacisnął Enter bez żadnego znaczącego wejścia.
- – Czy muszę w ogóle używać ferror ()? fgetc zwraca już EOF, gdy coś poszło nie tak, ale po prostu przestań czytać ze strumienia, zamiast mówić użytkownikowi, że coś poszło nie tak.
- – Nie dzieje się to za każdym razem, ale zdarza się: A \ n pozostaje w strumieniu stdin i następnym razem chcę uzyskać znaczące dane wejściowe fgetc () po prostu zostaje pominięty. Nie ma to znaczenia tutaj, ale ma znaczenie, gdy zadaję jedno znakowe pytania Tak / Nie. Mogę pozbyć się tego problemu tylko za pomocą konstrukcji, która usuwa wszystkie poprzednie rzeczy, które pozostają w stdin. Zobacz drugi blok kodu. .
Czy podchodzę do tego całkowicie źle? Czy są lepsze praktyki do przyjęcia? Wydaje mi się to naprawdę niezgrabne, a niezgrabne jest zawsze złe.
/** @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; } Drugi CodeBlock z prostszym przypadkiem:
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; } } Komentarze
Odpowiedź
Architektura
stdin jest zwykle buforowane po linii . Więc nic nie jest przekazywane do fgetc(), dopóki użytkownik nie naciśnie Enter . Kod OP wyświetli wiele komunikatów o błędach z danymi wejściowymi typu „Hello 123”. Lepiej oddzielić dane wejściowe użytkownika od sprawdzania poprawności danych wejściowych. Przeczytaj wiersz wprowadzania danych użytkownika za pomocą fgets() lub jakiejś własnej wersji, ponieważ fgets() ma kilka słabych stron. Następnie sprawdź poprawność danych wejściowych.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Jeśli chodzi o „Chciałbym pozbyć się zewnętrznej pętli while”. Ta pętla istnieje po to, aby dyskretnie konsumować "\n". Jeśli chcesz, aby zrobiła to pętla, po prostu poprzedz pętlę wewnętrzną
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
ch nie jest najlepszym typem. fgetc() zwraca zazwyczaj 257 różnych wartości [0-255] i EOF. Aby je odpowiednio rozróżnić, zapisz wynik w 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") { To samo dotyczy char tmp;
realloc()
Przesyłanie nie jest potrzebne.
Modyfikuj pod kątem braku pamięci, aby zwolnić string – niepotrzebne, jeśli kod po prostu się kończy, ale dobra praktyka przy umieszczaniu zabawek (kod „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; Dobre wykorzystanie sizeof(*strings) poniżej. Zalecane uproszczenie.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Dobre użycie size_t do przedstawienia rozmiaru tablicy. Co ciekawe, kod nie działa tak samo z int index;. Poleć size_t index;
is...()
Dzięki zastosowaniu int ch nie ma potrzeby przesyłania. Ponieważ th to test logiczny, zalecamy użycie ! zamiast arytmetycznego == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Poniższe mogą być łatwiejsze do zrozumienia – mniej negacji. (Problem ze stylem)
if (!(ch == " " || isalpha(ch))) { ferror()
Niezłe sprawdzenie if (ferror(in_stream))
Nazwy zmiennych
string, strings jest tak samo przydatne, jak wywołanie liczby całkowitej integer. Może zamiast tego phrase, dictionary.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() ma błędną nazwę. Brzmi ogólnie, ale kod ogranicza wprowadzanie do liter i spacji.Może get_words()?
Komentarze
- Mam wrażenie, że podałeś tę samą odpowiedź dwa razy? Tak czy inaczej, to jest odpowiedź, której szukałem! Byłem całkowicie skoncentrowany na używaniu fgetc sind fgets ' nt działało na początku (z powodu nieuczciwego \ ns w stdin). Wydaje się to o wiele lepsze i uwzględnię to w swoim kodzie. Dzięki!
- @Haini wiesz, że oprócz akceptacji, możesz również zagłosować za odpowiedź, jeśli tak bardzo Ci się spodobała 😉
Odpowiedź
Zła strategia realloc
Obecnie wywołujesz realloc() po każdym przeczytanym znaku. Powoduje to \ $ O (n ^ 2) \ $ czas na odczytanie ciągu, ponieważ za każdym razem, gdy wywołujesz realloc(), może być konieczne skopiowanie bieżącej zawartości do nowego bufora . Powinieneś albo po prostu przydzielić bufor o rozmiarze MAX_DATA, a następnie użyć realloc, aby zmniejszyć przydział na końcu, lub zmienić strategię realokacji gdzie rozmiar realokacji jest zwiększany za każdym razem o mnożnik (na przykład 2x).
Dotyczy to również tablicy ciągów, w której robisz to samo.
Dziwne wcięcie
Wcięcie jest dziwne, ponieważ zagnieżdżona pętla while znajduje się na tym samym poziomie wcięcia, co zewnętrzna pętla while.
Używać fgets ()?
Osobiście użyłbym fgets() (lub innej funkcji bibliotecznej, takiej jak readline()), aby odczytać ciąg. fgets() robi prawie to samo, co twoja pętla bez całej logiki zakodowanej ręcznie.
stdbool.h, ale nie ' nie próbuj tworzyć własnych wartości bools.