Tabela de pesquisa com matriz fixa

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) e remove(struct table*, const char*). A representação da tabela pode ser uma matriz de um struct par ou um par de matrizes (const char*[] e int*); 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

  • Como um detalhamento estilístico, eu ‘ d recomendo o * 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 um const char. Observe também que isso torna mais claro que tipo b está em int *a, b;.

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 em struct 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 a pair->val ou pair->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 retorna void *, e em C é válido converter void * 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 um malloc protótipo e o padrão é retornar um int (é uma convenção C antiga). Agora, se int 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). Prefira sizeof(expression). No seu caso, considere

     struct Pair * pair = malloc(sizeof(*pair)); 
  • table_insert insere cegamente em uma tabela inteira. Deve testar para tbl->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; } 

Deixe uma resposta

O seu endereço de email não será publicado. Campos obrigatórios marcados com *