Implementei o seguinte exercício:
Implementar um tabela de consulta com operações como
find(struct table*, const char*)
,insert(struct table*, const char*,int)
eremove(struct table*, const char*)
. A representação da tabela pode ser uma matriz de umstruct
par ou um par de matrizes (const char*[]
eint*
); você escolher, escolha também os tipos de retorno para suas funções.
#include <stdio.h> #include <stdlib.h> #include <string.h> #include <assert.h> #define ARR_SIZE 10 struct Pair { const char* word; int val; }; struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; struct Pair* make_pair(const char* word, int val) { assert(word); struct Pair* pair = (struct Pair*)malloc(sizeof(struct Pair)); pair->val = val; pair->word = word; return pair; } void table_empty(struct Table* tbl) { assert(tbl); size_t i = 0; for (i = 0; i < tbl->sz; ++i) { free(tbl->pairs[i]); tbl->pairs[i] = NULL; } tbl->sz = 0; } int search_word(struct Table* tbl, const char* word) { assert(tbl); assert(word); size_t i = 0; for (i = 0; i < tbl->sz; ++i) { //printf("%s %i\n", tbl->pairs[i]->word,i); if (strcmp(tbl->pairs[i]->word, word) == 0) { return i; } } return -1; // error } void table_insert(struct Table* tbl, const char* word, int val) { assert(tbl); assert(word); int i = search_word(tbl, word); if (i != -1) { // replace val tbl->pairs[i]->val = val; } else { // add new pair struct Pair* pair = make_pair(word, val); tbl->pairs[tbl->sz] = pair; // add pair at the last position ++tbl->sz; } } int table_find(struct Table* tbl, const char* word, int* return_val) { assert(tbl); assert(word); assert(return_val); int i = search_word(tbl, word); if (i != -1) { *return_val = tbl->pairs[i]->val; return 0; } return -1; // error not found } int table_remove(struct Table* tbl, const char* word) { assert(tbl); assert(word); int i = search_word(tbl, word); if (i == -1) return -1; free(tbl->pairs[i]); // free value at current pos tbl->pairs[i] = tbl->pairs[tbl->sz - 1]; // put address of last word at the pos of the current --tbl->sz; // "erase" last word return 0; } void table_print(struct Table* tbl) { assert(tbl); printf("\n"); printf("table size = %i\n", tbl->sz); for (int i = 0; i < tbl->sz; ++i) printf("%s %i\n", tbl->pairs[i]->word, tbl->pairs[i]->val); fflush(stdout); } void print_search_result(struct Table* tbl, const char* word) { assert(tbl); assert(word); int val = 0; if (table_find(tbl, word, &val) == 0) printf("%s %i\n",word, val); else printf("%s not found in table\n", word); printf("\n"); fflush(stdout); } void print_remove_result(struct Table* tbl, const char* word) { assert(tbl); assert(word); if (table_remove(tbl, word) == -1) printf("%s not deleted\n", word); else printf("%s deleted\n", word); printf("\n"); fflush(stdout); } int main() { struct Table table = { 0 }; int val = 0; table_insert(&table, "Hello", 10); table_insert(&table, "Test", 15); table_insert(&table, "Hello", 18); // testing overrite val table_insert(&table, "What", 5); table_insert(&table, "is", 3); table_insert(&table, "going", 4); table_insert(&table, "on", 77); table_print(&table); print_search_result(&table, "Hello"); print_search_result(&table, "Test"); print_search_result(&table, "keyword"); print_remove_result(&table, "Hello"); print_remove_result(&table, "Hello"); // double delete == false print_remove_result(&table, "What"); print_remove_result(&table, "going"); print_remove_result(&table, "is"); print_remove_result(&table, "on"); print_remove_result(&table, "Test"); table_print(&table); table_insert(&table, "Hello", 10); table_insert(&table, "Test", 15); table_insert(&table, "Hello", 18); // testing overrite val table_insert(&table, "What", 5); table_insert(&table, "is", 3); table_insert(&table, "going", 4); table_insert(&table, "on", 77); table_print(&table); table_empty(&table); table_print(&table); getchar(); }
Sinta-se à vontade para comentar sobre quaisquer melhorias. Existe uma maneira melhor de fazer isso?
Eu também tenho uma pergunta específica: meus usos de assert
são apropriados?
Comentários
Resposta
Usando struct
Pessoalmente, acho que você fez a escolha certa ao usar um struct
com char*
e int
em vez de 2 matrizes de char*
e int
, respectivamente. O Pair
é uma unidade conceitual de dados em seu aplicativo, então faz sentido colocá-los juntos. Se você tivesse 2 arrays separados, seria fácil para eles ficarem fora de sincronia e difícil depurar o motivo disso. Bom trabalho!
Preferir const
e funções para macros
Você definiu o tamanho do array usando uma macro. Isso coloca você em desvantagem durante a programação. Você removeu as informações de tipo para o valor. Ao torná-lo:
const int ARR_SIZE = 10;
você obtém segurança de digitação. Editar: Isso é “um C ++ – ismo que não” t trabalho em C. Meu mal! Mas o resto do conselho no próximo parágrafo está correto até onde eu sei.
Com macros que usam parâmetros, você corre o risco de serem usadas de maneiras inesperadas e causar problemas difíceis de depurar. Felizmente, você não fez isso aqui. Mas, em geral, se você estiver procurando por uma macro, pergunte-se se seria melhor com uma constante ou função. Quase sempre será (assumindo que pode usar a constante da maneira desejada).
Erros
Existem alguns erros no seu código. Em make_pair()
, você não verifica para ver se malloc()
foi bem-sucedido. Se falhar, nenhuma memória é alocada e pair
aponta para NULL
. Ao tentar atribuir pair->val
ou pair->word
, você travará.
Se você corrigiu isso, table_insert()
usa o resultado de make_pair()
sem verificar se “s NULL
primeiro. Isso ganhou” t travar imediatamente, porque você está apenas atribuindo o ponteiro da matriz em tbl->pairs[tbl->sz]
para ter o valor de pair
. O que acontecerá é mais tarde, quando você tentar pesquisar, imprimir ou inserir outro item, você “travará quando iterar sobre essa entrada na tabela e tentar fazer qualquer coisa com ela.
Você poderia tornar esses erros impossíveis se não alocar dinamicamente as entradas da matriz. Basta fazer do array um array de Pair
estruturas em vez de um ponteiro para eles.
Nomenclatura
Muitos dos seus nomes são realmente bons . Pair
e Table
são nomes decentes e legíveis para esta tarefa. make_pair()
, table_insert()
, etc. são informativos. Alguns poderiam ser melhorados, no entanto. tbl
não economiza muito digitar em table
. Basta usar a palavra inteira. O mesmo acontece com sz
vs. size
. i
é aceitável como um nome de variável de loop, mas seria ainda melhor se fosse mais descritivo. Por exemplo, entry_index
ou pair_index
. Ele deve descrever o que você está repetindo.
Comentários
- quando eu mudo a definição para
const int ARR_SIZE = 10;
me dá um erro emstruct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; };
que ARR_SIZE não é const. Então, como usá-lo neste caso? - Desculpe – que ‘ sa C ++ – ismo. Achei que fosse uma pergunta C ++. Meu erro. ‘ atualizarei minha resposta.
- Eu ‘ um grande defensor de nomes de variáveis descritivos, mas com a simplicidade dos loops neste programa, acho que o nome da variável
i
funciona bem. - Se
malloc()
falhar, atribuindo apair->val
oupair->word
pode travar, se você tiver sorte . Ele pode continuar e dar resultados errados ou pode fazer algo totalmente inesperado. Essa ‘ é a alegria do comportamento indefinido!
Resposta
-
Não lance o resultado de
malloc
.malloc
retornavoid *
, e em C é válido convertervoid *
em qualquer ponteiro. Se você lançar para suprimir um aviso sobre a conversão de inteiro em ponteiro, significa que um compilador não tem ummalloc
protótipo e o padrão é retornar umint
(é uma convenção C antiga). Agora, seint
e o ponteiro forem de tamanhos diferentes, o valor de retorno do malloc será truncado, com todas as consequências desagradáveis. -
Não é recomendado
sizeof(type)
. Prefirasizeof(expression)
. No seu caso, considerestruct Pair * pair = malloc(sizeof(*pair));
-
table_insert
insere cegamente em uma tabela inteira. Deve testar paratbl->sz < ARR_SIZE
e retorna uma indicação de erro se não for. -
A inserção real
struct Pair* pair = make_pair(word, val); tbl->pairs[tbl->sz] = pair; // add pair at the last position
deve ser realmente uma única linha:
tbl->pairs[tbl->sz] = make_pair(word, val);
Comentários
- por que não é uma boa ideia lançar malloc? Em c ++, até obteria um erro do compilador?
- @Sa ndro4912 A questão está marcada como C. C ++ é uma linguagem diferente, particularmente nesse aspecto. Agora, se o compilador C reclamar, significa que o
malloc
protótipo está faltando, o que pode levar a problemas desagradáveis. - sim, eu sei é c. Eu estava me perguntando por que aqui não é uma boa prática indicar as alterações de tipo com um elenco
- @ Sandro4912 Ver edição.
- @ Sandro4912
pair = malloc(sizeof *pair)
é mais fácil de codificar corretamente na primeira vez, mais fácil de revisar e manter.
Resposta
Habilitar todos avisos do compilador
Com um compilador bem habilitado, for (int i = 0; i < tbl->sz; ++i)
deveria ter avisado sobre a mistura de sign-ness de i
e tbl->sz
, bem como intervalo. Economize tempo e ative todos os avisos e use for (size_t i = 0; i < tbl->sz; ++i)
.
Em geral, o código é mole ao usar int,size_t
quase que indistintamente . Fiz uma nova arquitetura e usei size_t
apenas.
Uso misto de cópia superficial e profunda
make_pair(const char* word, int val)
aloca e forma um novo struct Pair
(cópia profunda), mas não copia o que word
aponta para.
Talvez
// pair->word = word; pair->word = strdup(word);
Use const
search_word()
não modifica *tbl
, então use const to convey that. Same for
table_find () ,
table_print () ,
print_search_result () `.
// int search_word(struct Table* tbl, const char* word) int search_word(const struct Table* tbl, const char* word)
Nomenclatura
Código usa const char* word;
, mas não é uma “palavra”, mas uma string conforme usada em strcmp()
.
—– Adições
Violação do contrato?
Nada nos requisitos “Implementar uma tabela de pesquisa com operações como …” indica que const char*
é um ponteiro para uma string . Chamando strcmp()
é questionável, a menos que existam requisitos não declarados. Como char *
, o código pode usar uma comparação simples
// if (strcmp(tbl->pairs[i]->word, word) == 0) if (tbl->pairs[i]->word == word)
Uso de assert()
meus usos de assert são apropriados?
Se o char *
ponteiro para adicionar / pesquisar não é especificado como uma string , assert(word);
não é adequado como word == NULL
não é conhecido como inválido.
O assert(return_val)
em table_find(struct Table* tbl, const char* word, int* return_val)
está OK, mas eu iria redesenhar para permitir return_val == NULL
if (i != -1) { // add if (return_val) { *return_val = tbl->pairs[i]->val; } return 0; }
*
em um ponteiro tipo adjacente ao nome da variável (por exemplo,const char *word
). A sintaxe de tipo C foi projetada para imitar o uso, onde, por exemplo, você ‘ d digite*word
para acessar umconst char
. Observe também que isso torna mais claro que tipob
está emint *a, b;
.