Jai lu lentrée utilisateur de stdin en clair C. Le problème est que je veux une implémentation saine, robuste aux erreurs et restreignant lutilisateur à une certaine entrée et ne « pas sucer en termes de complexité. La fonction get_strings() lit le caractère dentrée par car tant quil ny a pas de nouvelle ligne (\n), pas de EOF et que tous les caractères passent le isalpha() test. Mais je veux garder des espaces.
Certains points qui (je pense) méritent une attention particulière lors de lexamen:
- – Jadorerais me débarrasser du boucle while externe qui teste essentiellement si lutilisateur vient dappuyer sur Entrée sans aucune entrée significative.
- – Dois-je même utiliser ferror ()? fgetc renvoie déjà EOF quand quelque chose ne va pas, mais je vais juste arrêter la lecture du flux au lieu de dire à lutilisateur que quelque chose sest mal passé.
- – Cela ne se produit pas à chaque fois mais cela arrive: A \ n reste dans le flux stdin et la prochaine fois que je veux obtenir une entrée significative fgetc () est simplement ignoré. Cela na pas dimportance ici, mais cest le cas lorsque je pose des questions Oui / Non à un seul caractère. Je ne peux me débarrasser de ce problème quavec une construction qui efface toutes les choses précédentes laissées dans stdin. Voir le deuxième bloc de code pour cela .
Alors, est-ce que je gère totalement ce problème? Y a-t-il de meilleures pratiques à adopter? Cela me semble vraiment maladroit, et maladroit est toujours mauvais.
/** @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 avec un cas plus simple:
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; } } Commentaires
Réponse
Architecture
stdin est généralement tamponné en ligne . Ainsi, rien nest donné à fgetc() tant que lutilisateur nappuie pas sur Entrée . Le code OP donnera plusieurs messages derreur avec une entrée comme « Hello 123 ». Mieux vaut séparer lentrée utilisateur de la validation dentrée. Lisez la ligne dentrée utilisateur avec fgets() ou une version de votre choix car fgets() a quelques faiblesses. Ensuite validez lentrée.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } Concernant « jaimerais me débarrasser de la boucle while externe ». Cette boucle existe pour consommer silencieusement "\n". Si vous voulez quune boucle fasse cela, juste précédée de la boucle interne avec
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
Le ch nest pas le meilleur type. fgetc() renvoie généralement 257 valeurs différentes [0-255] et EOF. Pour bien les distinguer, enregistrez le résultat dans un 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") { Idem pour char tmp;
realloc()
Cast pas besoin.
Modifiez pour manque de mémoire pour libérer string – pas nécessaire si le code sort simplement, mais bonne pratique pour mettre vos jouets (code « s pointeur).
// 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; Bonne utilisation de sizeof(*strings) ci-dessous. Recommander une simplification.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Bonne utilisation de size_t pour représenter une taille de tableau. Curieusement, le code ne fait pas la même chose avec int index;. Recommander size_t index;
is...()
En utilisant int ch, pas besoin de cast. Depuis e Il sagit dun test logique, nous vous recommandons dutiliser ! plutôt que larithmétique == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { Ce qui suit peut être plus facile à comprendre – moins de négations. (Problème de style)
if (!(ch == " " || isalpha(ch))) { ferror()
Belle vérification de if (ferror(in_stream))
Les noms de variables
string, strings est aussi utile que appeler un entier integer. Peut-être que phrase, dictionary à la place.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() est mal nommé. Cela semble général, mais le code limite la saisie aux lettres et à lespace.Peut-être que get_words()?
Commentaires
- Jai limpression que vous avez posté la même réponse deux fois? Bref, cest la réponse que je cherchais! Jétais totalement concentré sur lutilisation de fgetc sind fgets ‘ nt fonctionnait pas au début (à cause des \ ns voyous dans stdin). Cela semble bien meilleur et je vais incorporer cela dans mon code. Merci!
- @Haini vous savez quen plus daccepter, vous pouvez également voter pour une réponse si vous lavez tellement aimé 😉
Réponse
Mauvaise stratégie de réallocation
Actuellement, vous appelez realloc() sur chaque caractère que vous lisez. Il en résulte un temps \ $ O (n ^ 2) \ $ pour lire une chaîne, car chaque fois que vous appelez realloc(), il peut être nécessaire de copier le contenu actuel dans le nouveau tampon . Vous devez soit allouer simplement un tampon de taille MAX_DATA, puis utiliser pour réduire lallocation à la fin, soit passer à une stratégie de réallocation. où la taille de la réallocation est augmentée à chaque fois dun facteur multiplicatif (par exemple 2x).
Cela sapplique également à votre tableau de chaînes, où vous faites la même chose.
Indentation étrange
Votre indentation est étrange car votre boucle while imbriquée est au même niveau dindentation que la boucle externe while.
Utiliser fgets ()?
Personnellement, jutiliserais fgets() (ou une autre fonction de bibliothèque telle que readline()) pour lire une chaîne. fgets() fait à peu près ce que fait votre boucle sans toute la logique codée à la main.
stdbool.h, mais ne ‘ nessayez pas de lancer vos propres booléens.