Implementé el siguiente ejercicio:
Implemente un tabla de búsqueda con operaciones como
find(struct table*, const char*)
,insert(struct table*, const char*,int)
yremove(struct table*, const char*)
. La representación de la tabla podría ser una matriz de unstruct
par o un par de matrices (const char*[]
yint*
); 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
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 enstruct 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 apair->val
opair->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
devuelvevoid *
, y en C es válido convertirvoid *
a cualquier puntero. Si lanza para suprimir una advertencia sobre la conversión de entero a puntero, significa que un compilador no tiene unmalloc
prototipo, y por defecto devuelve unint
(es una antigua convención de C). Ahora, siint
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)
. Prefierasizeof(expression)
. En su caso, considerestruct Pair * pair = malloc(sizeof(*pair));
-
table_insert
inserta a ciegas en una tabla completa. Debe probartbl->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; }
*
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 unconst char
elemento. Tenga en cuenta también que esto deja más claro qué tipob
es enint *a, b;
.