Tabla de búsqueda con arreglo fijo

Implementé el siguiente ejercicio:

Implemente un tabla de búsqueda con operaciones como find(struct table*, const char*), insert(struct table*, const char*,int) y remove(struct table*, const char*). La representación de la tabla podría ser una matriz de un struct par o un par de matrices (const char*[] y int*); que elija, también elija tipos de retorno para sus funciones.

#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(); } 

No dude en comentar sobre cualquier mejora. ¿Existe una mejor manera de hacer esto?

También tengo una pregunta específica: ¿son apropiados mis usos de assert?

Comentarios

  • Como detalle estilístico, ‘ recomiendo * en un puntero type sea adyacente al nombre de la variable (por ejemplo, const char *word). La sintaxis de tipo C se diseñó para imitar el uso, donde, por ejemplo, ‘ escribe *word para acceder a un const char elemento. Tenga en cuenta también que esto deja más claro qué tipo b es en int *a, b;.

Responder

Usando struct

Personalmente, creo que tomó la decisión correcta al usar un struct con char* y int en lugar de 2 matrices de char* y int, respectivamente. Pair es una unidad conceptual de datos en tu aplicación, por lo que tiene sentido juntarlos. Si tuviera 2 matrices separadas, sería fácil para ellas perder la sincronización y difícil depurar por qué sucedió eso. ¡Buen trabajo!

Prefiero const y funciones a macros

Has definido el tamaño de la matriz usando una macro. Esto pone usted está en desventaja durante la programación. Ha eliminado la información de tipo para el valor. Al hacerlo:

const int ARR_SIZE = 10; 

obtienes seguridad de tipos. Editar: Eso es «un C ++ – ismo que no» t trabajo en C. ¡Mi mal! Pero el resto del consejo en el siguiente párrafo es correcto hasta donde yo sé.

Con macros que toman parámetros, corre el riesgo de que se utilicen de forma inesperada y provoquen problemas difíciles de depurar. Afortunadamente, no lo ha hecho aquí. Pero en general, si se encuentra buscando una macro, pregúntese si estaría mejor con una constante o una función. Casi siempre lo estará (suponiendo que pueda usar la constante de la forma deseada).

Errores

Hay algunos errores en su código. En make_pair(), no comprueba para ver si malloc() tuvo éxito. Si falla, no se asigna memoria y pair apunta a NULL. Cuando intentas asignar pair->val o pair->word, fallarás.

Si lo arreglaste, table_insert() usa el resultado de make_pair() sin verificar si es NULL primero. Esto ganó » No se bloquee de inmediato, porque simplemente está asignando el puntero de matriz en tbl->pairs[tbl->sz] para que tenga el valor de pair. Lo que sucederá más tarde cuando si intenta buscar, imprimir o insertar otro elemento, se bloqueará cuando repita esa entrada en la tabla e intente hacer algo con ella.

Podría hacer que estos errores no sean posibles si no asignar dinámicamente las entradas de la matriz. Simplemente haga que la matriz sea una matriz de Pair estructuras en lugar de un puntero a ellas.

Nombrar

Muchos de sus nombres son realmente buenos . Pair y Table son nombres decentes y legibles para esta tarea. make_pair(), table_insert(), etc. son informativos. Sin embargo, algunos podrían mejorarse. tbl no le ahorra mucho escribir sobre table. Solo usa la palabra completa. Lo mismo ocurre con sz frente a size. i es aceptable como nombre de variable de bucle, pero sería incluso mejor si fuera más descriptivo. Por ejemplo, entry_index o pair_index. Debe describir lo que «estás repitiendo.

Comentarios

  • cuando cambio la definición a const int ARR_SIZE = 10; me da un error en struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; que ARR_SIZE no es constante. Entonces, ¿cómo usarlo en este caso?
  • Lo siento, ‘ sa C ++ – ismo. Pensé que esta era una pregunta de C ++. Mi error. Yo ‘ actualizaré mi respuesta.
  • Yo ‘ soy un firme partidario de los nombres de variables descriptivas, pero con la simplicidad de los bucles en este programa, creo que el nombre de la variable i funciona bien.
  • Si malloc() falla, la asignación a pair->val o pair->word podría bloquearse, si tiene suerte . Podría simplemente continuar y dar resultados incorrectos, o podría hacer algo totalmente inesperado. ¡Ese ‘ es el gozo del comportamiento indefinido!

Respuesta

  • No transmita el resultado de malloc.

    malloc devuelve void *, y en C es válido convertir void * a cualquier puntero. Si lanza para suprimir una advertencia sobre la conversión de entero a puntero, significa que un compilador no tiene un malloc prototipo, y por defecto devuelve un int (es una antigua convención de C). Ahora, si int y el puntero son de diferente tamaño, el valor de retorno de malloc se truncará, con todas las desagradables consecuencias. p>

  • No se recomienda sizeof(type). Prefiera sizeof(expression). En su caso, considere

     struct Pair * pair = malloc(sizeof(*pair)); 
  • table_insert inserta a ciegas en una tabla completa. Debe probar tbl->sz < ARR_SIZE y devuelve una indicación de error si no lo es.

  • La inserción real

     struct Pair* pair = make_pair(word, val); tbl->pairs[tbl->sz] = pair; // add pair at the last position 

    debería ser realmente una sola línea:

     tbl->pairs[tbl->sz] = make_pair(word, val); 

Comentarios

  • ¿por qué no es una buena idea lanzar malloc? ¿En c ++ incluso obtendría un error de compilación?
  • @Sa ndro4912 La pregunta tiene la etiqueta C. C ++ es un lenguaje diferente, particularmente en este sentido. Ahora, si el compilador de C se queja, significa que falta el prototipo malloc, lo que puede ocasionar problemas desagradables.
  • Sí, sé que es c. Me preguntaba por qué aquí no es una buena práctica indicar los cambios de tipo con un elenco
  • @ Sandro4912 Ver edición.
  • @ Sandro4912 pair = malloc(sizeof *pair) es más fácil de codificar correctamente la primera vez, más fácil de revisar y mantener.

Responder

Habilitar todos advertencias del compilador

Con un compilador bien habilitado, for (int i = 0; i < tbl->sz; ++i) debería haber advertido sobre la mezcla de sign-ness de i y tbl->sz así como el rango. Ahorre tiempo y habilite todas las advertencias y use for (size_t i = 0; i < tbl->sz; ++i).

En general, el código es blando al usar int,size_t casi indistintamente . Rediseñé y utilicé size_t únicamente.

Uso mixto de copia superficial y profunda

make_pair(const char* word, int val) asigna y forma un struct Pair (copia profunda) completamente nuevo, pero no copia lo que word apunta.

Quizás

// pair->word = word; pair->word = strdup(word); 

Use const

search_word() no modifica *tbl, así que 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) 

Denominación

Código usa const char* word; pero no es una «palabra», sino una cadena como se usa en strcmp().

—– Adiciones

¿Violación del contrato?

Nada en los requisitos «Implementar una tabla de búsqueda con operaciones como …» indica que const char* es un puntero a una cadena . Por lo tanto, llamar a strcmp() es cuestionable a menos que existan requisitos no declarados. Como char *, el código podría usar una comparación simple

// if (strcmp(tbl->pairs[i]->word, word) == 0) if (tbl->pairs[i]->word == word) 

Uso de assert()

¿Son apropiados mis usos de assert?

Si el char * puntero para agregar / buscar no está especificado para ser una cadena , assert(word); no es apropiado como word == NULL no es válido.

El assert(return_val) en table_find(struct Table* tbl, const char* word, int* return_val) está bien, pero lo rediseñaría para permitir return_val == NULL

if (i != -1) { // add if (return_val) { *return_val = tbl->pairs[i]->val; } return 0; } 

Deja una respuesta

Tu dirección de correo electrónico no será publicada. Los campos obligatorios están marcados con *