次の演習を実装しました:
実装
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
の使用は適切ですか?
コメント
回答
struct
の使用
個人的には、 struct
には、iv id =の2つの配列ではなく、char*
とint
が含まれています。それぞれ “eccc22e211”>
とint
。Pair
はアプリ内のデータの概念的な単位であるため、それらをまとめることは理にかなっています。 2つの別々のアレイがある場合、それらが同期しなくなるのは簡単で、なぜそれが起こったのかをデバッグするのは難しいでしょう。よくできました!
const
と関数をマクロよりも優先します
マクロを使用して配列サイズを定義しました。これでプログラミング中に不利になります。値の型情報を削除しました。作成することで:
const int ARR_SIZE = 10;
型安全性が得られます。 編集:それは「C ++-主義ではない」ということです。 Cで動作します。私の悪い!しかし、次の段落の残りのアドバイスは、私が知る限り正しいものです。
パラメーターを受け取るマクロを使用すると、予期しない方法で使用され、デバッグが困難になるリスクがあります。幸いなことに、ここではそれを行っていません。しかし、一般に、マクロに手を伸ばす場合は、定数または関数のどちらを使用したほうがよいかを自問してください。ほとんどの場合、(定数を使用できると仮定して)
エラー
コードにいくつかのエラーがあります。make_pair()
では、確認をチェックしません。 malloc()
が成功した場合。失敗した場合、メモリは割り当てられず、pair
はNULL
を指します。 pair->val
またはpair->word
を割り当てようとすると、クラッシュします。
修正した場合は、table_insert()
は、make_pair()
の結果を、最初にNULL
かどうかを確認せずに使用します。これが勝ちました。 ” tbl->pairs[tbl->sz]
の配列ポインタにpair
の値を割り当てているだけなので、すぐにクラッシュします。後で何が起こるかは別のアイテムを検索、印刷、または挿入しようとすると、「テーブル内のそのエントリを繰り返し処理して何かを実行しようとすると、クラッシュが発生します。
これらのエラーは、次の方法では不可能になる可能性があります。配列エントリを動的に割り当てます。配列を、それらへのポインタではなく、Pair
構造体の配列にするだけです。
命名
多くの名前は本当に良いです。 Pair
とTable
は、このタスクの適切で読みやすい名前です。 make_pair()
、table_insert()
などが参考になります。ただし、改善できるものもあります。 tbl
では、table
を入力する手間が省けます。単語全体を使用してください。 sz
とsize
は同じです。 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
の結果をキャストしないでください。malloc
はvoid *
であり、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)
はi
とi
の符号の混合について警告する必要があります。 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; }
を許可するように再設計します
*
を推奨します変数名の隣に入力します(例:const char *word
)。 Cタイプの構文は、使用法を模倣するように設計されています。たとえば、’ dタイプ*word
で要素。これにより、b
がint *a, b;
に含まれるタイプがより明確になることにも注意してください。