Eu li a entrada do usuário de stdin em C. O problema é que eu quero uma implementação sã que seja robusta a erros e restrinja o usuário a uma determinada entrada e não seja péssima em termos de complexidade. A função get_strings() lê o caractere de entrada por char, desde que não haja uma nova linha (\n), não EOF e todos os chars estão passando no isalpha() teste. Mas eu quero manter os espaços.
Alguns pontos que (eu acho) merecem atenção especial durante a revisão:
- – Eu adoraria me livrar do loop while externo, que basicamente testa se o usuário pressionou Enter sem nenhuma entrada significativa.
- – Eu preciso usar ferror ()? fgetc já retorna EOF quando algo deu errado, mas vou apenas pare de ler o fluxo em vez de dizer ao usuário que algo deu errado.
- – Isso não acontece sempre, mas acontece: A \ n permanece no fluxo stdin e na próxima vez eu quero obter uma entrada significativa fgetc () é simplesmente ignorado. Não importa aqui, mas importa quando estou fazendo perguntas de Sim / Não com um único caractere. Só posso me livrar desse problema com uma construção que limpa todas as coisas anteriores que foram deixadas em stdin. Veja o segundo bloco de código para isso .
Então, estou lidando com isso de forma totalmente errada? Existem práticas melhores para abraçar? Parece muito desajeitado para mim, e desajeitado é sempre ruim.
/** @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; } Segundo CodeBlock com um caso mais simples:
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; } } Comentários
Resposta
Arquitetura
stdin geralmente é linha em buffer . Portanto, nada é fornecido a fgetc() até que o usuário pressione Enter . O código OP fornecerá várias mensagens de erro com entrada como “Hello 123”. Melhor separar a entrada do usuário da validação de entrada. Leia a linha de entrada do usuário com fgets() ou alguma versão sua, pois fgets() tem alguns pontos fracos. Em seguida, valide a entrada.
char *input; while ((input = my_gets()) != NULL) { if (valid_phrase(input)) { foo(input); } else { fprintf(stderr, "Invalid input\n"); } free(input); } A respeito de “Eu adoraria me livrar do loop while externo”. Esse loop existe para consumir "\n" silenciosamente. Se você quiser que um loop faça isso, apenas preceda o loop interno com
int ch; while ((ch = fgetc()) == "\n") ; ungetc(ch, stdin); char ch
O ch não é o melhor tipo. fgetc() retorna normalmente 257 valores diferentes [0-255] e EOF. Para distingui-los corretamente, salve o resultado em um 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") { O mesmo para char tmp;
realloc()
O elenco não é necessário.
Modifique por falta de memória para liberar string – não necessário se o código simplesmente sair, mas uma boa prática para colocar seus brinquedos (código “s ponteiro).
// 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; Bom uso de sizeof(*strings) abaixo. Recomende simplificação.
strings = (char**) realloc(strings, (index+1)*sizeof(*strings)); strings = realloc(strings, sizeof *strings * (index+1)); size_t len
Bom uso de size_t para representar um tamanho de matriz. Curiosamente, o código não faz o mesmo com int index;. Recomendar size_t index;
is...()
Com o uso de int ch, não há necessidade de elenco. é um teste lógico, recomendo usar ! em vez de aritmética == 0.
// if (ch != " " && isalpha((int)ch) == 0) { if (ch != " " && !isalpha(ch)) { O seguinte pode ser mais fácil de entender – menos negações. (Problema de estilo)
if (!(ch == " " || isalpha(ch))) { ferror()
Boa verificação de if (ferror(in_stream))
Nomes de variáveis
string, strings é tão útil quanto chamar um número inteiro integer. Talvez phrase, dictionary em vez disso.
// OK /** @brief Contains the dictionary */ static char **strings = NULL; // Better // comment not truly needed static char **dictionary = NULL; get_strings() está com o nome incorreto. Parece geral, mas o código restringe a entrada a letras e espaço.Talvez get_words()?
Comentários
- Acho que você postou a mesma resposta duas vezes? Enfim, essa é a resposta que eu estava procurando! Eu estava totalmente focado em usar o fgetc sind. O fgets ‘ não funcionou no início (por causa do rogue \ ns no stdin). Isso parece muito melhor e vou incorporar isso no meu código. Obrigado!
- @Haini você sabe que além de aceitar, você também pode votar positivamente em uma resposta se gostou tanto 😉
Resposta
Estratégia de realloc ruim
Atualmente, você chama realloc() em cada caractere que lê. Isso resulta em um \ $ O (n ^ 2) \ $ tempo para ler uma string, porque toda vez que você chamar realloc(), pode ser necessário copiar o conteúdo atual para o novo buffer . Você deve apenas alocar um buffer de tamanho MAX_DATA e então usar realloc para reduzir a alocação no final, ou mudar para uma estratégia de realocação onde o tamanho de realocação é aumentado por um fator multiplicativo a cada vez (como 2x).
Isso também se aplica ao seu array de strings, onde você faz a mesma coisa.
Indentação estranha
Sua indentação é estranha porque seu loop while aninhado está no mesmo nível de indentação que o loop externo while.
Use fgets ()?
Eu pessoalmente usaria fgets() (ou alguma outra função de biblioteca, como readline()) para ler uma string. fgets() faz praticamente o que seu loop faz sem toda a lógica codificada manualmente.
stdbool.h, mas não ‘ tente lançar seus próprios bools.