- ベストアンサー
リンクリスト、deleteRecord() の書き方は?
以下のような関数がありまして、 リストの真ん中と最後を削除することは できるのですが 先頭がうまくいきません。 また、先頭の場合は free() をどのように使うのでしょうか? code を見直してみると読み易くないような気もするんですが、 もっと効率の良い書き方ありますでしょうか? int deleteRecord(struct record **start, char name[]) { int judge = 0; struct record *temp, *temp2; temp = *start; if(debugmode == 1) printf("deleteRecord called.\n"); if(temp != NULL) { if(strcmp(temp->name, name) == 0) { if(temp->next == NULL) { /* only start point */ temp = NULL; } else { temp = temp->next; } judge = 1; } else if(temp->next != NULL) { while(temp->next != NULL) { if(strcmp(temp->next->name, name) == 0) { if(temp->next->next != NULL) { free(temp->next); temp->next = temp->next->next; } else { free(temp->next); temp->next = NULL; } judge = 1; } if(temp->next != NULL) temp = temp->next; } } } return judge; }
- みんなの回答 (4)
- 専門家の回答
質問者が選んだベストアンサー
同じstrcmpを三箇所でやってるのはよくないですね。 DRY(Don't Repeat Yourself)でいきましょう。 即興で書いたんで多分バグがいますし、もうちょっと書き直せますがとりあえず。 #一回無駄な strcmpが入るのはご愛嬌。工夫すれば避けられますけどね。 struct record { char name[16]; struct record *next; }; define strEQ(x, y) (strcmp((x), (y)) == 0) int delete_record2(struct record **start, char name[]) { struct record *p; struct record head; int judge = 0; //if (!*start) // return judge; head.name[0] = '\0'; head.next = *start; p = &head; while (p) { if (strEQ(p->name, name)) { struct record *tmp; tmp = p; p = p->next; free(tmp); judge = 1; continue; } p = p->next; } return judge; } あと気になる点を。 同じ値を持つものは重複登録される可能性があり、削除のときは 同じ値を持つものはすべて削除というのは管理のやり方として どうかなという気がします。 #リストを全部なめないといけないから まあ登録時に重複登録をチェックしても同様の手間になりますが。
その他の回答 (3)
- Tacosan
- ベストアンサー率23% (3656/15482)
とりあえず, 「レコードを検索する」関数と「レコードを削除する関数」を分けた方がいいんじゃないかなぁ. 例えば検索する関数を struct record **findRecord(struct record **start, const char name[]); 削除する関数を int deleteRecord(struct record **item); と宣言しておく, みたいな. deleteRecord の中身は難しくなく, 基本的には if (item != NULL && *item != NULL) { struct record *temp = *item; *item = temp->next; killRecord(temp); } で終わり. あ, killRecord は「そのレコードのデータを (必要なら中身の free も行って) 全部削除して, そのレコードそのものも free する」関数.
- ape5
- ベストアンサー率57% (85/148)
先頭については、*start != NULL なので if(temp!=NULL){ if(strcmp(temp->name,name)==0){ temp2=temp->next; start = &temp2; free(temp); temp=temp2; }else ・・・・・・・・ } コードは何度も推敲しているうちに単純化できるところに気づいていきます。 がんばってください。
- sakusaker7
- ベストアンサー率62% (800/1280)
とりあえずヒントだけ。 リストの先頭に、実データ(この例でいえば name)を持たないリンクだけの 「いけにえ」を置くようにすれば、リストの(実際の)先頭要素を特別扱いしないで済むようにできます。 ところで > free(temp->next); > temp->next = temp->next->next; temp->next を free しているのに、その後で temp->next->next なんてアクセスしてはダメです。環境によっては動くかもしれませんが、 動かなくても文句は言えません。
補足
返事が遅くなり申し訳ないです。 みなさんのヒントを参考に、ようやく一通り 動くものが出来たのですが どうでしょうか? int deleteRecord(struct record **start, char name[]) { struct record *temp, *temp2; int judge = 0; temp = temp2 = *start; if(debugmode == 1) printf("[ deleteRecord called. ]\n\n"); if(*start != NULL) { while((temp->next != NULL) && (strcmp((*start)->name, name) == 0)) { *start = (*start)->next; free(temp); temp = temp2 = *start; judge = 1; } while(temp->next != NULL) { if(strcmp(temp->name, name) == 0) { temp2->next = temp->next; free(temp); temp = temp2; judge = 1; } temp2 = temp; temp = temp->next; } if(strcmp(temp->name, name) == 0) { /* if only one list */ temp2->next = NULL; free(temp); judge = 1; if(*start == temp) *start = NULL; } } return judge; }
お礼
> 削除のときは同じ値を持つものはすべて削除というのは 管理のやり方としてどうかなという気がします。 それがこの課題の「仕様」なんで今回はこれで良いんです^^ DRY なんて法則があるんですね。 とても勉強になりました。 丁寧に教えていただき ほんとにありがとうございます!!