paulo1205
(usa Ubuntu)
Enviado em 05/09/2018 - 09:40h
Nosso colega AnsiC, acima, já explicou a origem do
bug .
Permita-me fazer alguns comentários sobre o código, de coisas que me parecem um tanto estranhas.
• Aquela
struct Status , que tem um campo só, e que tem aspecto de apenas conter um valor booleano. Precisa dessa complicação toda? E de fazer algo como “
c[flag].situacao.disponivel ”, em lugar de simplesmente algo como
c[flag].disponivel .
• Por falar na expressão usada como exemplo acima,
flag também não é um nome muito bom porque tem o sentido de informação booleana, mas o que você tem ali é um índice.
• Os nomes das funções estão inconsistentes. Se suas funções para iniciar e para listar o
array têm os nomes
iniciar e
listar (verbos no infinitivo), por que a função que tem o propósito de cadastrar ficou com o nome
cadastro (substantivo)? E se essas três têm seus nomes em Português, por que
findPos ,
string e
flush_in têm os seus em Inglês? E por que
findPos destaca os nomes das palavras usando a forma de
camel casing , ao passo que
flush_in usa separadores entre as palavras e todas as letras minúsculas?
• Outros elementos do código também estão inconsistentes no uso de seus nomes: algumas variáveis e tipos de dados estão em Inglês, outras em Português.
• O nome
string , em particular, é uma escolha bem estranha, pois ele é quase universalmente usado para indicar um tipo de dados (ainda que não nativo em C), não uma função. Seria melhor algo como
read_string (ou
readString , ou ainda
ler_cadeia ).
• Nos parâmetros dessa mesma função, você usa duas formas distintas para declarar
prompt e
str , embora as duas sejam sinônimas, produzindo como tipo resultante
char * ), e usa o atributo
const para o último parâmetro, que seria desnecessário, uma vez que, quando a função for invocada, o valor passado como argumento para esse parâmetro será uma cópia do valor da expressão. Como a função não vai alterar o valor de
prompt , que é passado como referência (i.e. ponteiro), faria sentido colocar para ela o atributo
const . Desse modo, um protótipo melhor para essa função (já também com um nome mais apropriado) provavelmente seria algo como “
void read_string(const char *prompt, char *str, size_t str_size) ”.
• A declaração de
main não está de acordo com o que prescreve o padrão do C. Da forma como você fez, a função poderia receber qualquer quantidade de argumentos de quaisquer tipos. O padrão da linguagem oferece duas opções válidas para a declaração de
main , e você deve escolher entre elas de acordo com sua disposição de receber argumentos do sistema operacional na hora em que o programa for executado. Se você quiser receber esses argumentos, você deve declarar a função como “
int main(int argc, char **argv) ” (os nomes
argc e
argv podem ser diferentes, mas não seus tipos);
argc indica a quantidade de argumentos passados pelo sistema operacional, e
argv contém ponteiros para cada um desses argumentos, cada um deles armazenado na forma de uma
string de caracteres (por convenção,
argv[0] indica o nome usado para invocar o programa, e
argv[argc] contém um ponteiro nulo). Se, por outro lado, você não tiver interesse em receber argumentos do sistema operacional (que parece ser o caso do seu programa), deve usar a a forma “
int main(void) ”. [Em C++, ao contrário do C, uma lista de parâmetros vazia numa declaração significa que a função não aceita nenhum argumento, então a forma que você usou poderia ser aceita. Contudo, você está usando C, não C++; então, deve seguir as regras do C.]
•
sizeof(char) é sempre, por definição, igual a 1. Desse modo, ele é necessariamente redundante, podendo ser suprimido em todas as suas ocorrências.
• Outra observação sobre aquele indicador de se o registro está disponível ou não é que ele está com um sentido invertido: você usa
0 , que em C tem sentido booleano de
falso para indicar que que a condição de disponível é verdadeira, e
1 , que em C normalmente tem o sentido de
verdadeiro , para indicar a condição de disponibilidade falsa. Isso torna seu código anti-intuitivo (por exemplo: em
findPos , você interrompe prematuramente o laço de repetição de busca por registro disponível quando encontra a condição “
!p[i ].situacao.disponivel ”). Se você simplesmente mudar o nome da condição de “
disponivel ” para algo como “
em_uso ”, essa inconsistência e anti-intuitividade desaparece.
• Ao usar
malloc e
realloc , você não precisa converter explicitamente o valor de retorno para o tipo de ponteiro da variável que vai receber esse valor de retorno, porque toda conversão de
void * é automaticamente conversível para qualquer outro tipo de ponteiro. Por ser redundante, tal conversão vira uma coisa a mais para dar manutenção, e, portanto, mais um potencial lugar para se introduzirem
bugs . [Em C++ essa conversão automática não existe. Mas se você usar C++, é improvável que deva usar
malloc e
realloc : ou você usaria o operador
new ou um dos tipos de
containers da biblioteca padrão, tais como
std::vector ,
std::list etc., ou mesmo
std::string .]
• Por que ler a
string para um
array auxiliar, alocado dinamicamente, e depois copiá-lo para o lugar definitivo, em vez de ler diretamente para o lugar definitivo?
• O uso de
realloc está errado. Como a realocação é sujeita a falhar, e sinaliza falhas através de um ponteiro nulo como valor de retorno, mas ao mesmo tempo garante que o ponteiro original será preservado em caso de falha, você não deveria usar como destino da atribuição do valor de retorno a mesma variável usada como argumento. A forma correta de fazer seria a seguinte.
// Ponteiro original em ‘ptr’, e tamanho original em ‘ptr_size’.
void *new_ptr=realloc(orig_ptr, new_size);
if(new_ptr){
ptr=new_ptr;
ptr_size=new_size;
}
else{
// Faz o tratamento do erro de alocação, se for o caso.
}
• De modo geral, você não está tratando devidamente nenhuma operação que possa resultar em erro. Deveria fazê-lo para todas as alocações e realocações, bem como para todas as operações de entrada de dados.
• No caso da leitura de
strings , os erros possíveis não são apenas aqueles erros detectados pelo sistema operacional e pela biblioteca, mas também os casos em que a entrada digitada excede o tamanho máximo.
• De modo geral, em operações de leitura, é essencial testar o valor de retorno das funções de leitura. Além disso, quando se usa
scanf , algo que pode ser muito útil é a pseudoconversão
%n , que serve para reconhecer a quantidade de caracteres consumidos pela função até o momento.
• Não use
flush_in (nem qualquer função parecida, quer na biblioteca, quer sua, quer de terceiros), a não ser que você tenha certeza de que já há elementos que não lhe interessam na entrada. Como, em muitos casos, a presença de conteúdo a ser descartado pode depender daquilo que o usuário digitar, quase sempre essas funções de descarte (ou de releitura dos dados) terão de ser cercadas ou precedidas de
if s cujas condições avaliem o estado da entrada após as operações de leituras mais recentes. Caso contrário, você
pode obrigar o usuário a gerar conteúdo inútil (mesmo que seja apenas uma apertada de
Enter a mais, ainda será a mais), apenas para ser descartado em seguida.
• Sua função
flush_in tem uma falha de implementação: se ocorrer um erro de leitura (por exemplo: fim de arquivo ou algum erro proveniente do sistema operacional), o valor retornado pela função nunca será igual a
'\n' e, portanto, a função entrará num
loop infinito. A forma correta de fazer seria a seguinte.
int c; // Tem de ser int, não char!
do c=getchar(); while(c!=EOF && c!='\n');