元のコードでは、このフラグメントは疑わしいものでした:
if(current->head!=NULL){
hx = current->head->grade;
cy = current->current->grade;}
else{
}
if(current->head==NULL){
インデントは、それ自体の行の右中括弧が侵入者であることを示唆しています。空のブロックは必要ありませんelse。インデントは、コードが新しい条件のセットではなくif (current->head == NULL)、本体の一部であることを示しています。else侵入者として識別された行を削除する場合、関数の最後に追加の右中括弧が必要になる可能性があります。
コードをきれいにインデントすると、ブロックがどのように並んでいるかを簡単に確認できます。インデントをめちゃくちゃにすると、このようにエラーを見つけるのが難しくなります。インデントは、コードを読みやすく理解しやすくするために使用されます。誤解を招くインデントは、コードが読みにくく、理解して正しく理解するのが難しいことを意味します。
改訂されたコードでは、元のコメントが取り除かれ、読みやすくするために再フォーマットおよびインデントされ、議論のポイントのために新しいコメントで注釈が付けられています。
void add_course(STUDENT *current)
{
int hx,cy,tmpz; // P1
COURSE *tmp = current->current; // P2
if (current->head != NULL) // P3
{
hx = current->head->grade;
cy = current->current->grade;
}
if (current->head == NULL)
{
current->head = current->current; // P4
current->tail = current->current;
current->tail->next = NULL; // P5
}
else
{
if (hx > cy) // P6
{
current->current->next = current->head;
current->head = current->current;
current->current = current->head;
while (current->current->next != NULL)
current->current = current->current->next;
current->tail->next = current->current;
current->tail = current->current;
current->tail->next = NULL;
}
else
{ // P7
current->current = current->head;
hx = current->current->grade;
tmpz = tmp->grade;
while (current->current->next != NULL && tmpz > hx)
{
current->current = current->current->next;
hx = current->current->next->grade;
}
current->current->next = tmp->next;
current->current->next = tmp;
current->current = tmp;
}
}
}
ブロックの開き中括弧の位置は議論の余地があります。ifK&R を含め、else、while、for、 、 などと同じ行にある左中括弧を好む人がいます。私はインデントのオールマン スタイルswitchを強く好みますが、必要に応じてローカルの慣例を使用します。私が何かを大幅に再フォーマットすると、それはオールマン スタイルになります。この変更は、コードに対する批判の一部ではありません。
コース レコードのない学生レコードがどのように表されるかを示していません。最も妥当な表現は、 、head、tailおよびcurrentがすべて NULL に初期化されていることです。
あなたの関数は学生の記録を取りますが、(驚くべきことに) という名前にもかかわらず、コースを追加する必要はありませんadd_course()。at P4の使用を考えると、関数を呼び出す前に新しいコースにcurrent->current設定することで部分的にコースを追加したと仮定する必要があり、とcurrent->currentで始まるリスト内の新しいコースの位置を修正する関数が必要です。headで仕上げtailます。これは機能的にあまりまとまりがなく、設計が不十分です。理想的には、新しいコース レコードを別の引数として関数に渡し、nextフィールドを NULL にする必要があります (コース名を初期化し、成績を初期化する必要があります)。
void add_course(STUDENT *student, COURSE *new_course);
P1 と P2 の変数は P6 または P7 まで使用されないことがわかります。そのため、P1、P2、および P3 のコードは P6 またはそれ以降に移動する必要があります。C99 (および C11 と C++) でサポートされている「どこでも定義」ルールを使用するのではなく、C89 コードに固執します。
P4 から P5 の段落は、空のリストを扱うことになっています。使用するように記述できますtmp(その場合、P2 の定義はそのままにしておく必要があります)。次のように書く方がわかりやすいかもしれません:
COURSE *new_course = current->current;
if (current->head == NULL)
{
current->head = new_course;
current->tail = new_course;
new_course->next = NULL;
}
関数が呼び出される前にコースが適切に初期化されている場合、最終的な割り当ては不要です。
学生に関連付けられたコースが既に 1 つあり、呼び出しコードによって の値をcurrent->current新しいコースに変更し、この関数を呼び出すことによって、新しいコースが追加されたとします。考慮すべき状況がいくつか考えられます。
- 新しいコースは既存のコースの複製です (拒否する必要がありますが、関数は値を返さないため、コースが拒否されたことを示す方法はありません)。
- 新しいコースのグレードは、既存のコースのグレードよりも低くなります。
- 新しいコースのグレードは、既存のコースのグレードよりも高くなります。
- 新しいコースのグレードは、既存のコースのグレードと同じです。
この問題は、最初と最後の状況で何をすべきかを指定していません。2 番目の状況が発生した場合、新しいコースを既存のコースの前に挿入する必要があります。3 番目が発生した場合、新しいコースは既存のコースの後に挿入する必要があります。明確にするために、新しいグレードが同じ場合、コースはアルファベット順にリストする必要があります。重複を処理するには、一致するリスト全体を検索する必要があります。おそらく関数にカプセル化する必要があります。
COURSE *find_course(STUDENT *student, COURSE *course);
headこの関数は学生を取得し、 からまでのリストを検索してtail、コース名とリスト内のコースを比較します。リスト内に一致する要素があれば、その要素を返します。ここのコードでは、名前が見つからないことを示す NULL を関数が返すことだけが必要です。
COURSE *find_course(STUDENT *student, COURSE *course)
{
COURSE *next_student = student->head;
while (next_student != NULL && strcmp(next_student->cname, course->cname) != 0)
next_student = next_student->next;
return next_student;
}
この関数のインターフェイスと実装を次のように変更することは可能であり、おそらくより柔軟です。
COURSE *find_course(COURSE *course, const char *cname)
{
while (course != NULL)
{
if (strcmp(course->cname, cname) == 0)
return(course);
course = course->next;
}
return(course);
}
これを使用して、有効なコースのリストなど、適切に作成されたコースのリストを検索できます (無効なコースを拒否できます)。
また、コードの繰り返しを避けるために、複数の既存のコースがある場合にどうすべきかを確認する必要があります。重複コース チェックは同じで、最初に行う必要があります。add_course()帰納法により、空のリストが適切であり、単一の要素リストが適切であると安全に想定できるため、コースのリストが常に適切であると判断できます。
しかし、私はあなたが取り組むためにそれを残すつもりです.
コース比較機能が必要になります。と同じ規則を使用できますstrcmp()。最初の引数が 2 番目の引数の前に来る場合は負の数を返し、2 番目の引数が最初の引数の前に来る場合は正の数を返します。2 つのコースが同じ場合は (名目上) ゼロ:
int cmp_course(const COURSE *c1, const COURSE *c2)
{
if (c1->grade < c2->grade)
return -1;
else if (c1->grade > c2->grade)
return +1;
else
return(strcmp(c1->cname, c2->cname));
}
継続
[...長い一時停止...24時間以上経過...]
これは、コメント解除され、動作し、コンパイルされ、実行中の (C99) プログラムにラップされたコードです。軽度の再フォーマットとアサーションの追加以外は、コードをまったく変更していませんadd_course()。
#include <assert.h>
#include <inttypes.h>
#include <stdio.h>
typedef struct student
{
char sname[32];
char sid[9];
struct student *snext;
struct course *current;
struct course *head;
struct course *tail;
} STUDENT;
typedef struct course
{
char cname[15];
char grade;
struct course *next;
} COURSE;
extern void add_course(STUDENT *current);
void add_course(STUDENT *current)
{
int hx,cy,tmpz;
COURSE *tmp = current->current;
assert(tmp != 0);
if (current->head != NULL)
{
hx = current->head->grade;
cy = current->current->grade;
}
if (current->head == NULL)
{
current->head = current->current;
current->tail = current->current;
current->tail->next = NULL;
}
else
{
if (hx > cy)
{
current->current->next = current->head;
current->head = current->current;
current->current = current->head;
while (current->current->next != NULL)
current->current = current->current->next;
current->tail->next = current->current;
current->tail = current->current;
current->tail->next = NULL;
}
else
{
current->current = current->head;
hx = current->current->grade;
tmpz = tmp->grade;
while (current->current->next != NULL && tmpz>hx)
{
current->current = current->current->next;
hx = current->current->next->grade;
}
current->current->next = tmp->next;
current->current->next = tmp;
current->current = tmp;
}
}
}
static void dump_student(FILE *fp, const char *tag, const STUDENT *student)
{
fprintf(fp, "Student: %s\n", tag);
fprintf(fp, "Name: %s; ID: %s\n", student->sname, student->sid);
fprintf(fp, "Next: 0x%" PRIXPTR "\n", (uintptr_t)student->snext);
fprintf(fp, "Current: 0x%" PRIXPTR "; ", (uintptr_t)student->current);
fprintf(fp, "Head: 0x%" PRIXPTR "; ", (uintptr_t)student->head);
fprintf(fp, "Tail: 0x%" PRIXPTR "\n", (uintptr_t)student->tail);
COURSE *cp = student->head;
while (cp != 0)
{
fprintf(fp, "Course: %-14s (%c) (0x%.16" PRIXPTR ")\n",
cp->cname, cp->grade, (uintptr_t)cp->next);
cp = cp->next;
}
}
int main(void)
{
STUDENT s1 = { "Yours Truly", "me", 0, 0, 0, 0 };
COURSE c[] =
{
{ "Math", 'B', 0 },
{ "English", 'A', 0 },
{ "Science", 'D', 0 },
{ "History", 'C', 0 },
{ "French", 'C', 0 },
};
dump_student(stdout, "Before", &s1);
for (int i = 0; i < 5; i++)
{
char buffer[8];
sprintf(buffer, "After%d", i+1);
s1.current = &c[i];
add_course(&s1);
dump_student(stdout, buffer, &s1);
}
return(0);
}
関数に注意してくださいdump_student()。この種のインターフェースを備えた関数は非常に便利であり、後でデバッグするためにコードで使用できるようにしておくことがよくあります。このFILE *引数は、出力を標準エラー (またはログ ファイル) に送信できること、およびどの呼び出しが実行されているかを識別するためのタグを意味します。必要に応じて、ファイル、行、関数名をインターフェイスに追加できます。私は通常それをしません。
コードが C99 である場所は数カ所しかありません。のforループmain()と のコース ポインタの定義dump_student()。C コンパイラが C99 構文をサポートしていない場合は、変数定義を移動できます。
これは、Mac OS X 10.7.4 での 64 ビット コンパイルの出力例です。
Student: Before
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x0; Head: 0x0; Tail: 0x0
Student: After1
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x7FFF643D84E0; Head: 0x7FFF643D84E0; Tail: 0x7FFF643D84E0
Course: Math (B) (0x0000000000000000)
Student: After2
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x7FFF643D84E0; Head: 0x7FFF643D84F8; Tail: 0x7FFF643D84E0
Course: English (A) (0x00007FFF643D84E0)
Course: Math (B) (0x0000000000000000)
Student: After3
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x7FFF643D8510; Head: 0x7FFF643D84F8; Tail: 0x7FFF643D84E0
Course: English (A) (0x00007FFF643D84E0)
Course: Math (B) (0x00007FFF643D8510)
Course: Science (D) (0x0000000000000000)
Student: After4
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x7FFF643D8528; Head: 0x7FFF643D84F8; Tail: 0x7FFF643D84E0
Course: English (A) (0x00007FFF643D84E0)
Course: Math (B) (0x00007FFF643D8528)
Course: History (C) (0x0000000000000000)
Student: After5
Name: Yours Truly; ID: me
Next: 0x0
Current: 0x7FFF643D8540; Head: 0x7FFF643D84F8; Tail: 0x7FFF643D84E0
Course: English (A) (0x00007FFF643D84E0)
Course: Math (B) (0x00007FFF643D8540)
Course: French (C) (0x0000000000000000)
最初の数回の挿入は問題ありませんが、その後に問題があることに注意してください。私が実行しvalgrindたところ、コードの正常性が明確になりました (ただし、システム ライブラリの外部に動的メモリ割り当てはありません)。
3 回目の挿入後にリストが適切に拡張されない理由を突き止めることをお勧めします。