固定配列のルックアップテーブル

次の演習を実装しました:

実装find(struct table*, const char*)insert(struct table*, const char*,int)remove(struct table*, const char*)などの操作を含むルックアップテーブル。テーブルの表現は、structペアの配列、または配列のペア(const char*[]int*);選択し、関数の戻り値の型も選択します。

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

改善点についてコメントしてください。これを行うためのより良い方法はありますか?

1つの特定の質問もあります:assertの使用は適切ですか?

コメント

  • 文体的な落とし穴として、私は’ポインターに*を推奨します変数名の隣に入力します(例:const char *word)。 Cタイプの構文は、使用法を模倣するように設計されています。たとえば、’ dタイプ*wordで要素。これにより、bint *a, b;に含まれるタイプがより明確になることにも注意してください。

回答

struct

の使用

個人的には、 structには、iv id =の2つの配列ではなく、char*intが含まれています。それぞれ “eccc22e211”>

intPairはアプリ内のデータの概念的な単位であるため、それらをまとめることは理にかなっています。 2つの別々のアレイがある場合、それらが同期しなくなるのは簡単で、なぜそれが起こったのかをデバッグするのは難しいでしょう。よくできました!

constと関数をマクロよりも優先します

マクロを使用して配列サイズを定義しました。これでプログラミング中に不利になります。値の型情報を削除しました。作成することで:

const int ARR_SIZE = 10; 

型安全性が得られます。 編集:それは「C ++-主義ではない」ということです。 Cで動作します。私の悪い!しかし、次の段落の残りのアドバイスは、私が知る限り正しいものです。

パラメーターを受け取るマクロを使用すると、予期しない方法で使用され、デバッグが困難になるリスクがあります。幸いなことに、ここではそれを行っていません。しかし、一般に、マクロに手を伸ばす場合は、定数または関数のどちらを使用したほうがよいかを自問してください。ほとんどの場合、(定数を使用できると仮定して)

エラー

コードにいくつかのエラーがあります。make_pair()では、確認をチェックしません。 malloc()が成功した場合。失敗した場合、メモリは割り当てられず、pairNULLを指します。 pair->valまたはpair->wordを割り当てようとすると、クラッシュします。

修正した場合は、table_insert()は、make_pair()の結果を、最初にNULLかどうかを確認せずに使用します。これが勝ちました。 ” tbl->pairs[tbl->sz]の配列ポインタにpairの値を割り当てているだけなので、すぐにクラッシュします。後で何が起こるかは別のアイテムを検索、印刷、または挿入しようとすると、「テーブル内のそのエントリを繰り返し処理して何かを実行しようとすると、クラッシュが発生します。

これらのエラーは、次の方法では不可能になる可能性があります。配列エントリを動的に割り当てます。配列を、それらへのポインタではなく、Pair構造体の配列にするだけです。

命名

多くの名前は本当に良いです。 PairTableは、このタスクの適切で読みやすい名前です。 make_pair()table_insert()などが参考になります。ただし、改善できるものもあります。 tblでは、tableを入力する手間が省けます。単語全体を使用してください。 szsizeは同じです。 iはループ変数名として受け入れられますが、より説明的であるとさらに良いでしょう。たとえば、entry_indexまたはpair_indexです。何を繰り返しているかを説明する必要があります。

コメント

  • 定義をconst int ARR_SIZE = 10; struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; };で、ARR_SIZEがconstではないというエラーが表示されます。この場合、どのように使用しますか?
  • 申し訳ありませんが’ sa C ++-ism。これはC ++の質問だと思いました。悪いです。’回答を更新します。
  • I ‘記述変数名の強力なサポーターですが、このプログラムのループが単純なため、変数名iは問題なく動作します。
  • malloc()が失敗した場合は、pair->valまたはpair->word クラッシュする可能性があります運が良ければ。続行して間違った結果が得られる場合もあれば、まったく予期しないことを行う場合もあります。それは’未定義の振る舞いの喜びです!

回答

  • mallocの結果をキャストしないでください。

    mallocvoid *であり、Cではvoid *を任意のポインターに変換することが有効です。整数からポインタへの変換に関する警告を抑制するためにキャストした場合、コンパイラには「mallocプロトタイプがなく、デフォルトで(これは古代のC規則です)。intとポインタのサイズが異なると、mallocの戻り値が切り捨てられ、すべての厄介な結果が生じます。

  • sizeof(type)はお勧めしません。sizeof(expression)をお勧めします。この場合は、

     struct Pair * pair = malloc(sizeof(*pair)); 
  • table_insertは盲目的に完全なテーブルに挿入します。

    で、そうでない場合はエラー表示を返します。

  • 実際の挿入

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

    実際には1行である必要があります:

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

コメント

  • mallocをキャストするのは良い考えではないのはなぜですか?c ++ではコンパイラエラーが発生することさえありますか?
  • @Sa ndro4912質問にはCのタグが付けられています。C++は、特にこの点で異なる言語です。 Cコンパイラが文句を言う場合は、mallocプロトタイプがないことを意味し、厄介な問題につながる可能性があります。
  • はい。キャストで型の変更を示すのがなぜここでは良い習慣ではないのか疑問に思いました
  • @ Sandro4912編集を参照してください。
  • @ Sandro4912 pair = malloc(sizeof *pair)は、最初は正しくコーディングしやすく、確認と保守も簡単です。

回答

すべて有効にするコンパイラの警告

十分に有効化されたコンパイラを使用すると、for (int i = 0; i < tbl->sz; ++i)iiの符号の混合について警告する必要があります。 div id = “f64878db49”>

と範囲。時間を節約し、すべての警告を有効にして、for (size_t i = 0; i < tbl->sz; ++i)を使用します。

一般に、コードはint,size_tをほぼ同じ意味で使用するのが面倒です。 。再設計してsize_tのみを使用します。

浅いコピーと深いコピーの混合使用

make_pair(const char* word, int val)はまったく新しいstruct Pair(ディープコピー)を割り当てて形成しますが、wordが指すものはコピーしません。

おそらく

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

const

*tblを変更しないため、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) 

命名

コードconst char* word;を使用しますが、これは「単語」ではなく、strcmp()で使用される文字列です。

—–追加

契約違反?

「…などの操作でルックアップテーブルを実装する」という要件には、文字列へのポインタです。したがって、strcmp()は、明記されていない要件が存在しない限り、疑わしいものです。 char *として、コードは単純な比較を使用できます

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

assert()

アサートの使用は適切ですか?

追加/検索へのchar *ポインタが文字列として指定されていません。assert(word);として適切ではありません。 div id = “559ccc5a1b”> が無効であることがわかっていません。

table_find(struct Table* tbl, const char* word, int* return_val) divのassert(return_val) >は問題ありませんが、return_val == NULL

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

を許可するように再設計します

コメントを残す

メールアドレスが公開されることはありません。 * が付いている欄は必須項目です