paulo1205
(usa Ubuntu)
Enviado em 11/07/2019 - 10:38h
Saddler escreveu:
É verdade que é considerado uma má prática de programação C alocar memória em uma função e desalocar em outra função?
Nunca ouvi falar disso. E, sendo um dos usos de ponteiros justamente o de permitir que objetos alocados numa parte do programa possam ser passados a outra parte, tal ideia se torna ainda mais estranha.
Fico curioso de saber de que fonte você ouviu isso.
Tenho essa dúvida porque recentemente tive que fazer o uso deste recurso em um projeto que estou desenvolvendo sozinho e eu sinceramente não sei se foi uma boa abordagem ...
Segue os códigos abaixo para uma análise:
//trecho de código
struct art_properties{
int art;
int quote;
int art_color;
};
//trecho de código
static void handler(int num){
clear();
endwin();
init_scr();
}
int resize(int key){
Você não usa o parâmetro
key ao longo da função. Por que tê-lo, ent
struct sigaction new_action;
struct sigaction old_action;
new_action.sa_flags=0;
new_action.sa_handler=handler;
sigemptyset(&new_action.sa_mask);
sigaction(SIGWINCH, &new_action, &old_action);
return getmaxy(stdscr);
}
static size_t count_rows(const char *file_path){
size_t rows=0;
FILE *file=fopen(file_path, "r");
if(file!=NULL){
while(!feof(file)){
if(fgetc(file)=='\n'){
rows++;
}
}
fclose(file);
}
return rows;
}
struct art_properties *print_art(int y, struct art_properties *properties){
struct art_properties aux;
FILE *art_file=NULL, *quote_file=NULL;
size_t i=0;
char row_content[1025], str_quote[1025];
const char *arts[]={"obj/arts/art_001.txt", "obj/arts/art_002.txt",
"obj/arts/art_003.txt", "obj/arts/art_004.txt",
"obj/arts/art_005.txt", "obj/arts/art_006.txt",
"obj/arts/art_007.txt", "obj/arts/art_008.txt",
"obj/arts/art_009.txt", "obj/arts/art_010.txt"};
if(properties==NULL){
srand(time(NULL));
Normalmente você só chama
srand () uma vez ao longo da vida do programa. E uma forma de garantir isso é colocar essa chamada dentro de
main (), e fora de qualquer laço de repetição.
Eu não sei quantas vezes esta função aqui será chamada. Mas se for mais de uma vez, então esse
srand () nessa posição não é uma boa ideia.
aux.art=rand()%9;
aux.art_color=rand()%7;
Escolha de cores me leva a crer que você deveria usar
8 , em lugar de
7 . A não ser que você queira evitar a cor cinza/branca. É o caso?
Existem várias implementações de
rand ()/
srand (), mas a implementação tradicional tem uma baixa aleatoriedade dos bits de mais baixa ordem. Isso faz com que a operação de obtenção de resto para obter um valor limitado nem sempre seja muito boa, especialmente com divisores que sejam potências de 2 ou seus múltiplos.
Existem implementações melhores de
rand ()/
srand (), que melhoram a qualidade dos números pseudo-aleatórios] (PRN, de
pseudo random numbers ) gerados, mas mesmo com um algoritmo não tão bom, é possível melhorar a aleatoriedade do valor final por meio do deslocamento dos bits mais altos do PRN para baixo. Uma forma de fazer isso é a seguinte.
int color=(int)(((double)rand()/(1.0+(double)RAND_MAX))*8.0);
aux.quote=1+(rand()%count_rows("obj/quotes.txt"));
De novo, não sei quantas vezes você vai invocar
print_art (). Mas se for mais de uma, talvez fosse melhor você não contar a quantidade de linhas desse arquivo dentro da função, mas antes de ela ser chamada a primeira vez. Alternativamente, você poderia deixar dentro da função, mas atribuindo seu valor uma vez só, com o auxílio de variáveis locais, mas estáticas, conforme o exemplo abaixo.
void func(int arg1, char *arg2){
static int *p_quant_linhas=NULL; // Variável estática, inicializada apenas uma vez e com valor preservado ao longo de múltiplas chamadas a ‘func’.
if(!p_quant_linhas){
p_quant_linhas=malloc(sizeof *p_quant_linhas);
if(!p_quant_linhas){
perror("Erro de alocação de memória");
exit(1);
}
*p_quant_linhas=conta_linhas("arquivo");
}
// A partir deste ponto, ‘*p_quant_linhas’ terá a quantidade de linhas do arquivo.
// ...
int aux=1+(int)(((double)rand()/(1.0+(double)RAND_MAX))*(double)(*p_quant_linhas));
// ...
}
properties=malloc(sizeof(struct art_properties));
memcpy(properties, &aux, sizeof(struct art_properties));
Nas duas operações acima, eu acho ruim o uso do nome do tipo como argumento de
sizeof : além de mais verborrágico, dificulta a manutenção do código (caso algum dia, por qualquer razão, o tipo do ponteiro seja alterado, você teria de catar todas as ocorrências de operações sobre ele para fazer com que o tipo continue concordando).
Eu prefiro fazer de um modo que faça com que o compilador infira o tamanho a partir do tipo que já foi declarado para o próprio objeto que é alvo da operação.
properties=malloc(sizeof *properties);
memcpy(properties, &aux, sizeof *properties);
Mas, além disso, o caso acima ainda tem os seguintes pontos de atenção:
• Alocações podem falhar. Então você deveria testar o valor de
properties após a invocação de
malloc (), para ter certeza de que é válido gravar dados na região para onde tal valor aponta.
• Não é necessário usar
memcpy () para copiar valores de estruturas do mesmo tipo. Você poderia ter dito simplesmente “
*properties=aux; ” (desde que
properties aponte para um endereço válido, como acima mencionado).
}else{
memcpy(&aux, properties, sizeof(struct art_properties));
Aqui também caberia simplificar: “
aux=*properties; ”.
}
attron(COLOR_PAIR(aux.art_color) | A_BOLD);
art_file=fopen(arts[aux.art], "r");
if(art_file!=NULL){
for(i=0; i<count_rows(arts[aux.art]); i++){
fgets(row_content, 1025, art_file);
mvprintw(y+i, 1, "%s", row_content);
}
fclose(art_file);
}
quote_file=fopen("obj/quotes.txt", "r");
if(quote_file!=NULL){
if(aux.quote!=1){
for(int i=0; i!=aux.quote; i++){
fgets(str_quote, 1025, quote_file);
}
mvprintw(y+2+i, 1, "[GANADO QUOTE] - %s", str_quote);
}
fclose(quote_file);
}
attroff(COLOR_PAIR(aux.art_color) | A_BOLD);
return properties;
}
struct art_properties *free_art(struct art_properties *ptr){
free(ptr);
return NULL;
Eu não gosto muito desse jeito de fazer, porque você acaba tendo, na hora de usar a função, de colocar duas vezes o nome da variável ponteiro que será alterada pela desalocação (e.g. “
ptr_atr=free_art(ptr_art); ”.
Não é estritamente necessário alterar o valor de um ponteiro que acabou de ser desalocado para
NULL . A única efetiva proteção que isso traz é impedir que o valor antigo do ponteiro possa ser abusado para tentar observar conteúdo que ainda possa existir na memória, o que não é grande coisa, pois o mesmo efeito poderia ser conseguido se houver outra variável ponteiro com o mesmo valor, o que não é tão raro de acontecer. Por isso mesmo, a função padrão
free () não se dá ao trabalho de fazer isso.
Se você quiser realmente uma função de desalocação que force o ponteiro, seria mais confortável fazê-la da seguinte maneira.
void free_art(struct art_properties **ptr){
free(*ptr);
*ptr=NULL;
}
Com essa forma, você poderia invocá-la da seguinte maneira: “
free_art(&ptr_art); ”. Veja como ela é mais enxuta do que do outro jeito, referenciando a variável ponteiro uma só vez.
Entretanto, as duas versões fazem pouco mais do que servir de
wrapper para
free (), e parte desse pouco mais não é efetivamente útil. Assim sendo, por que não usar diretamente
free ()? Perceba que, ainda que a biblioteca padrão do C não o faça, em bibliotecas de uso muito comum, incluindo algumas do POSIX e SUS, diversas funções que fazem alocações de dados com finalidades específicas, tais como
strdup (),
getline (),
asprintf (), entre outras, não têm função específica para desalocação, mas esperam que a desalocação seja feita com
free (). Você é livre (sem trocadilho) para fazer o mesmo.
}
//trecho de código
case '0':
noecho();
do{
properties=print_art(1, properties);
attron(COLOR_PAIR(YELLOW) | A_BOLD);
mvprintw(y-1, 1, "[?] - Are you sure you want to quit Saddler [Y/n]?");
attroff(COLOR_PAIR(YELLOW) | A_BOLD);
key=getch();
Você não mostrou a declaração de
key . Um erro comum é a pessoa declarar variáveis que recebem caracteres e teclas como
char . Espero que não seja o seu caso, mas fica a informação de que o tipo tem de ser
int .
Quanto ao resto do código, achei a transformação de 'y' em 'Y' e de 'n' em 'N' muito extensa para pouco efeito. Dá para ser mais simples.
if(key>=0 && key<255){
unsigned char k=toupper(key);
if(k=='Y' || k=='N')
key=k;
}
y=resize(key);
clear();
switch(key){
case 'y':
case 'Y':
key='Y';
break;
case 'n':
case 'N':
key='N';
break;
default:
break;
}
}while(key!='Y' && key!='N');
properties=free_art(properties);
echo();
break;
... “Principium sapientiae timor Domini, et scientia sanctorum prudentia.” (Proverbia 9:10)