En utilisant struct

Personnellement, je pense que vous avez fait le bon choix en utilisant un struct avec un char* et int au lieu de 2 tableaux de char* et int, respectivement. Pair est une unité conceptuelle de données dans votre application, il est donc logique de les regrouper. Si vous aviez 2 tableaux séparés, il serait facile pour eux de se désynchroniser et difficile de déboguer pourquoi cela sest produit. Beau travail!

Préférez const et fonctions aux macros

Vous avez défini la taille du tableau à laide dune macro. Cela met vous êtes désavantagé lors de la programmation. Vous avez supprimé les informations de type pour la valeur. En le faisant:

const int ARR_SIZE = 10; 

vous obtenez la sécurité de type. Edit: Cest « un C ++ – ism that doesn » t travailler en C. Mon mauvais! Mais le reste des conseils du paragraphe suivant est correct pour autant que je sache.

Avec les macros qui prennent des paramètres, vous courez le risque de les utiliser de manière inattendue et de causer des problèmes difficiles à déboguer. Heureusement, vous n’avez pas fait cela ici. Mais en général, si vous vous trouvez à la recherche d’une macro, demandez-vous si vous seriez mieux avec une constante ou une fonction. Vous le serez presque toujours (en supposant que vous puissiez utiliser la constante de la manière souhaitée).

Erreurs

Il y a des erreurs dans votre code. Dans make_pair(), vous ne cochez pas pour voir si malloc() a réussi. En cas déchec, aucune mémoire nest allouée et pair pointe vers NULL. Lorsque vous essayez dattribuer pair->val ou pair->word, vous plantez.

Si vous avez résolu ce problème, table_insert() utilise le résultat de make_pair() sans vérifier sil « est NULL en premier. Cela a gagné » t planter immédiatement, car vous « êtes simplement en train dassigner le pointeur de tableau dans tbl->pairs[tbl->sz] pour avoir la valeur de pair. Ce qui se passera est plus tard lorsque vous essayez de rechercher, dimprimer ou dinsérer un autre élément, vous obtiendrez un plantage lorsque vous parcourez cette entrée dans le tableau et que vous essayez de faire quoi que ce soit avec.

Vous pourriez rendre ces erreurs impossibles à ne pas allouer dynamiquement les entrées du tableau. Faites simplement du tableau un tableau de structures Pair plutôt quun pointeur vers elles.

Dénomination

Beaucoup de vos noms sont vraiment bons . Pair et Table sont des noms décents et lisibles pour cette tâche. make_pair(), table_insert(), etc. sont informatifs. Certains pourraient cependant être améliorés. tbl ne vous évite pas beaucoup de taper sur table. Utilisez simplement le mot entier. Idem avec sz vs size. i est acceptable comme nom de variable de boucle, mais ce serait encore mieux sil était plus descriptif. Par exemple, entry_index ou pair_index. Il doit décrire ce sur quoi vous « réitérez ».

Commentaires

• lorsque je change la définition en const int ARR_SIZE = 10; cela me donne une erreur dans struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; que ARR_SIZE nest pas const, alors comment lutiliser dans ce cas alors?
• Désolé – que ‘ en C ++ – ism. Je pensais que cétait une question C ++. Ma mauvaise. Je ‘ je mettrai à jour ma réponse.
• Je ‘ je suis un fervent partisan des noms de variables descriptifs, mais avec la simplicité des boucles de ce programme, je pense que le nom de la variable i fonctionne très bien.
• Si malloc() échoue, attribuer à pair->val ou pair->word pourrait planter, si vous avez de la chance . Cela pourrait simplement continuer et donner de mauvais résultats, ou cela pourrait faire quelque chose de totalement inattendu. Cest ‘ la joie du comportement indéfini!

• Ne convertissez pas le résultat de malloc.

  malloc renvoie void *, et en C, il est valide de convertir void * en nimporte quel pointeur. Si vous effectuez un cast pour supprimer un avertissement concernant la conversion dentier en pointeur, cela signifie quun compilateur na pas de prototype malloc et quil renvoie par défaut un int (cest une ancienne convention C). Maintenant, si int et le pointeur sont de taille différente, la valeur de retour malloc serait tronquée, avec toutes les conséquences désagréables.

• Il nest pas recommandé de sizeof(type). Préférez sizeof(expression). Dans votre cas, envisagez

   struct Pair * pair = malloc(sizeof(*pair)); 

• table_insert insère aveuglément dans un tableau complet. Il doit tester tbl->sz < ARR_SIZE et renvoie une indication derreur si ce nest pas le cas.

• Linsertion réelle

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

  doit être en fait une seule ligne:

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

Commentaires

• pourquoi nest-ce pas une bonne idée de lancer malloc? En C ++, il obtiendrait même une erreur de compilation?
• @Sa ndro4912 La question est étiquetée C. C ++ est un langage différent, en particulier à cet égard. Maintenant, si le compilateur C se plaint, cela signifie que le prototype malloc est manquant, ce qui peut conduire à des problèmes désagréables.
• oui je sais que c. Je me suis simplement demandé pourquoi ici ce nest pas une bonne pratique dindiquer les changements de type avec un cast
• @ Sandro4912 Voir edit.
• @ Sandro4912 pair = malloc(sizeof *pair) est plus facile à coder correctement la première fois, plus facile à réviser et à gérer.

Tout activer avertissements du compilateur

Avec un compilateur bien activé, for (int i = 0; i < tbl->sz; ++i) aurait dû avertir du mélange de la signature de i et tbl->sz ainsi que range. Gagnez du temps et activez tous les avertissements et utilisez for (size_t i = 0; i < tbl->sz; ++i).

En général, le code est difficile à utiliser int,size_t presque de manière interchangeable . Je « réarchitecte et utilise size_t uniquement.

Utilisation mixte de copie superficielle et profonde

make_pair(const char* word, int val) alloue et forme un tout nouveau struct Pair (copie complète), mais ne copie pas ce que word pointe.

Peut-être

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

Utilisez const

search_word() ne modifie pas *tbl, utilisez donc 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) 

Dénomination

Code utilise const char* word; mais ce nest pas un « mot », mais une chaîne telle quutilisée dans strcmp().

—– Ajouts

Violation de contrat?

Rien dans les exigences « Implémenter une table de recherche avec des opérations telles que … » nindique que const char* est un pointeur vers une chaîne . Appelez donc strcmp() est discutable à moins dexigences non déclarées. En tant que char *, le code pourrait utiliser une simple comparaison

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

Utilisation de assert()

Mes utilisations dassert sont-elles appropriées?

Si le pointeur char * pour ajouter / rechercher nest pas spécifié comme étant une chaîne , assert(word); nest pas correct car word == NULL nest pas connu pour être invalide.

Le assert(return_val) dans table_find(struct Table* tbl, const char* word, int* return_val) est OK, mais je referais la conception pour autoriser return_val == NULL

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