前回で「完了」としたソースコードです.
#include <stdio.h> #include <stdlib.h> #include <string.h> #define PLAYER_MAX 20 enum result { /* 勝敗情報を表す列挙型 */ RSLT_NOTPLAYED, /* 未対戦 */ RSLT_WIN, /* 勝 */ RSLT_LOSE, /* 負 */ RSLT_NA /* (同一プレイヤー同士なので)対戦しない */ }; typedef struct { char player[PLAYER_MAX + 1]; int table[PLAYER_MAX][PLAYER_MAX]; int player_number; } rrtable; /* 文字列playerの中に同一文字が2つ以上あるか判定する */ int has_same_player(char *player) { int i, j; for (i = 0; i < strlen(player); i++) { for (j = i + 1; j < strlen(player); j++) { if (player[i] == player[j]) { return 1; } } } return 0; } rrtable *initialize(char *player) { rrtable *tab; int i; if (strlen(player) < 2) { printf("Too few players.\n"); exit(1); } if (strlen(player) > PLAYER_MAX) { printf("Too many players.\n"); exit(1); } if (has_same_player(player)) { printf("A player appears twice.\n"); exit(1); } if ((tab = (rrtable *)malloc(sizeof(rrtable))) == NULL) { printf("Malloc error.\n"); exit(1); } strcpy(tab->player, player); tab->player_number = strlen(player); memset((void *)tab->table, RSLT_NOTPLAYED, PLAYER_MAX * PLAYER_MAX); for (i = 0; i < tab->player_number; i++) { tab->table[i][i] = RSLT_NA; } return tab; } void print_table(rrtable *tab) { int i, j; printf(" %s\n", tab->player); for (i = 0; i < tab->player_number; i++) { putchar(tab->player[i]); for (j = 0; j < tab->player_number; j++) { switch (tab->table[i][j]) { case RSLT_NOTPLAYED: putchar(' '); break; case RSLT_WIN: putchar('o'); break; case RSLT_LOSE: putchar('x'); break; case RSLT_NA: putchar('-'); break; default: putchar('?'); } } putchar ('\n'); } } /* 標準入力から1行読み出して,lineの参照する配列に格納する */ char *get_line(char *line, int max_byte) { int c; int i; /* 1文字ずつ読み出して,lineの配列に格納する */ for (i = 0; i < max_byte && (c = getchar()) != EOF && c != '\n'; i++) { line[i] = c; } line[i] = '\0'; /* 改行になるまで読み飛ばす */ if (i == max_byte) { while ((c = getchar()) != EOF && c != '\n') ; } /* 1文字も読み出せなければNULLを返す */ if (line[0] == '\0') { return NULL; } return line; } /* 文字列playerから文字cを探してその位置を返す */ int search_player_index(rrtable *tab, char c) { int i; char *player = tab->player; for (i = 0; i < strlen(player); i++) { if (c == player[i]) { return i; } } /* 見つからなければ負の数を返す */ return -1; } /* 対戦結果を追加する */ void add_result(rrtable *tab, char *line) { int winner, loser; if (strlen(line) != 3 || line[1] != '>') { printf("%s: ignored.\n", line); return; } winner = search_player_index(tab, line[0]); loser = search_player_index(tab, line[2]); if (winner < 0 || loser < 0 || winner == loser) { printf("%s: ignored.\n", line); return; } tab->table[winner][loser] = RSLT_WIN; tab->table[loser][winner] = RSLT_LOSE; } int main(void) { #define LINE_BYTE_MAX 25 char line[LINE_BYTE_MAX + 1]; rrtable *tab; /* 最初の行を読み出し,構造体を初期化する */ get_line(line, LINE_BYTE_MAX + 1); tab = initialize(line); /* 各行を読み出し,表を埋めていく */ while (get_line(line, LINE_BYTE_MAX + 1)) { if (line[0] == '!') { print_table(tab); } else { add_result(tab, line); } } return 0; }
本日の作業は,ソースファイルの整理です.「清書」というほうがいいでしょう.試験やレポートを,書き殴ってそのまま提出することはありませんよね? プログラミングでも,動くものができたことに満足せず,全体を見直して,読みやすく保守しやすく,あわよくば再利用*1できるものにしておきたいわけです.
さて,上に挙げたファイル,どのように整理するかですが…箇条書きにしておきましょう.
- コメントをつける.
- 変数名,型,関数の戻り値を見直す.
- 同一の処理を共通化する.
- 関数プロトタイプをつける.
では順番に,見ていきますね.
「コメントをつける」…ファイルの先頭に,ファイル名,何をするプログラムか,そして作成者情報を書くのは,清書の第一歩です.複数行に渡るコメントの書き方にはいろいろありますが,ここでは簡潔なものを採用します.
/* * roundrobin.c - 総当たり戦の対戦結果を求める * * 作成者: takehikom * 作成日時: Thu May 17 05:30:06 JST 2007 */
ちなみにEmacsだと,「*」の縦位置を自動で揃えてくれます*2.
ほかにも,構造体型とその各メンバには,コメントをつけるべきでしょう.
typedef struct { /* 対戦表に関する構造体 */ char player[PLAYER_MAX + 1]; /* プレイヤー名 */ int table[PLAYER_MAX][PLAYER_MAX]; /* 勝敗 */ int player_number; /* プレイヤー数 */ } rrtable;
関数定義を順に見ていって,何をする関数なのか,コメントをつけておきたいものです.具体的なコメント内容は省略します.
それと,誤解を招くコメントa mesleading commentを修正しておきます.具体的にはsearch_player_index関数で,「/* 文字列playerから文字cを探してその位置を返す */」とありますが,「プレイヤー名の文字列から文字cを探してその位置を返す */」に変えておきます*3.
ただし,今回のプログラミングでは,コードから,どんな変数(情報を一時的に保持する場所)を使って,どんな処理をしているのかが分かるように心がけています.その分,コメントも少なめです.大規模なプログラムでは,関数のコメントを定型化して,名前,引数,戻り値,その他の説明をいちいち書くようなコメントスタイルになります.
「変数名,型,関数の戻り値を見直す」…変数にいちいちコメントをつけないにしても,あるいはつけるにしても,その使い方が適切かを確認します.名は体を表すということで,変数名を見ます.合わせて型も確認します.
今のコードで明らかにまずいわけではないのですが,あちこちに出現する「char *player」というのは,適切でないかもしれません.プレイヤーを表す情報は本来,1文字なので,char型であるべきです.となると,その文字列表現は,「char *player_string」あるいは「char *player_str」にすべきか…いずれも,なんとも人為的な,不自然な名称です.
プレイヤーの名前1文字を表す変数名に「player」を使っていたら,それは混乱の元ですが,そういうことがないように,書いてきています*4.ということで,このプログラムだけで見れば一貫性があるということで,「char *player」という表現はこのままにします.
型についても,プログラムを書いていく中で注意深く取り扱ったので,ここで要変更というのはありません.search_player_indexという関数について,その第1引数は,rrtable *tabではなく,char *playerとして,呼び出し側は「search_player_index(tab->player, …);」に変更したいようにも思えます.実際そう書き換えても,問題なく動きます.しかし現代のプログラミングにおいて,この種の手作業による最適化は好まれません.むしろ,構造体の中身を気にしなくても関数を呼び出せるようにするほうが重要でしょう.ということで,現状維持とします.
initialize関数の中でexit関数を多用しているのは,好ましくありません.構造体を動的に生成して,そのポインタを返すという関数では,失敗したらそこで即終了というよりは,NULLを返して,終了判定は呼び出し側でするほうが適切です.ということで,3箇所の「exit(1);」はいずれも「return NULL;」に置き換えます.そして,main関数内を一部修正します.「tab = initialize(line);」を,以下のコードに置き換えます.
if ((tab = initialize(line)) == NULL) { return 1; }
変数ではないのですが,main関数の中に書いている,マクロLINE_BYTE_MAXの#define文は,PLAYER_MAXの定義文の直後に移動しておきましょう.
「同一の処理を共通化する」…DRYという略語があります.Don't Repeat Yourselfの略で,重複排除の原則を意味します.関西弁で言うと,「おんなじことを,なんべんも言わすな」といったところでしょうか.
これまでのプログラムでも,同一の処理があれば,何らかの形で,共通化しておきましょう.別々に同じことを書いていると,間違いに気づいて一方だけを修正し,もう一方はそのままということがよく起こりますから.
ざっと見渡したところ,同一の処理があるのは,次の2箇所でした.
(1) get_line関数に2度書かれている「(c = getchar()) != EOF && c != '\n'」…標準入力から1文字取ってきて変数Cに格納し,EOFでも改行文字でもないことを確認しています.共通化の基本は,「関数」または「関数形式マクロ」ですが,ここでは関数形式マクロで書くことにします.
/* 1文字取得し,EOFでも改行でもなければ真 */ #define mygetchar(c) (((c) = getchar()) != EOF && (c) != '\n')
いい名前が思い浮かばなかったので,「俺getchar」の意味で「mygetchar」としました.関数形式マクロは,置換される引数と,置換内容全体にカッコをつけます.「*5」という表現は危なっかしい*6のですが,コンパイルしても,エラーも警告も出ません.
(2) add_result関数に2度書かれている「printf("%s: ignored.\n", line);」と「return;」…これも関数にしたいところですが,関数の呼び出し先で,add_result関数を終えようという,いわば多段returnというのはできませんし,関数プロトタイプの中にreturnを書くのも,スマートとは言えません.ここは関数呼び出しではなく,不正な入力を判定する,大きなif文を書くことで,解決を試みることにします.式が複雑なので,コメントをつけておきます.
/* 「勝者名>敗者名」でなければ,不正な入力とみなして無視する */ if (strlen(line) != 3 || line[1] != '>' || (winner = search_player_index(tab, line[0])) < 0 || (loser = search_player_index(tab, line[2])) < 0 || winner == loser) { printf("%s: ignored.\n", line); return; }
このifの条件式は,単に条件判定をしているのではなく,winnerとloserへの代入も行っています.これらの代入をif文の前に出すことは,しないほうがいいでしょう.lineを文字列としてみたとき,3文字未満の可能性があり,そのときloserに意味のない値が代入されるためです*7.条件が複数行になるとき,論理ORの「||」や論理ANDの「&&」を,行末に書くか次の行の最初に持ってくるか,悩むこともありますが,次の行の最初に書くのが,Cらしいでしょう.
「関数プロトタイプをつける」…これまで,関数プロトタイプの宣言をせずにやっていました.相互再帰をしない限り,関数プロトタイプなしでも,関数の定義の順番を配慮すれば,きちんと動くプログラムは書けます.しかしCという手続き型言語において,関数プロトタイプのないソースファイルは,目次のない卒業論文のようなもので,清書の際にはつけておくべきでしょう.
書く場所は分かっていますよね? 最初の関数定義,ここではhas_same_playerの直前です.
書き方は分かっていますよね? それぞれの関数定義の,最初の行をコピーして,行末にセミコロンをつけるだけです.慣習として,mainの関数プロトタイプは書きません.
int has_same_player(char *player); rrtable *initialize(char *player); void print_table(rrtable *tab); char *get_line(char *line, int max_byte); int search_player_index(rrtable *tab, char c); void add_result(rrtable *tab, char *line);
ここで,なぜ清書の段階で関数プロトタイプを書くべきかを明らかにしておきます.この「なぜ」は,プログラムを作っていく中で,関数定義をしたらすぐに関数プロトタイプを書くべきではないのはなぜか,と言い換えられます.
最初に「こんな機能がほしい」「一連の処理をさせたい」と関数を定義していても,その後,その引数の型,戻り値の型,あるいは関数名そのものを変更することがよく起こります.そのとき,関数定義とそのプロトタイプを両方書いていると,定義だけ書き換えて,関数プロトタイプの書き換えを忘れることも,よくあるものなのです.ということで,関数が変わり得る段階ではプロトタイプ宣言をしないでおいて,ほぼ固まった段階,すなわちソースファイルを整理している段階で書くほうがいいということです.
ここまで手を加えれば,自己採点でも85点くらいのソースファイルになるでしょう.
次回が最終回です.完成版のソースを確認するとともに,100点に近づけるためのアドバイスを手短にして,終えたいと思います.
*1:「再利用」は,コードの全部あるいは一部をそのまま他のプログラムに使うということに限りません.コードの全部あるいは一部を見て,その考え方を他のプログラミングに適用するということも,再利用とみなせるでしょう.
*2:Emacsで書く場合,「Time-stamp: <>」と書いておけば,ファイル保存時に,「<>」のところに作成日時とユーザ名が挿入されるので便利です.
*3:初期のコードでは,search_player_indexは,プレイヤー文字列を第1引数にとり,呼び出す際は,tab->playerを指定していたのでした.情報隠蔽の観点から,rrtableのポインタに変更して,コメントを変更し忘れていたのでした.
*4:プレイヤーの名前1文字を参照するのは,print_tableで行の最初に出力するときと,search_player_index関数を呼び出す際の第2引数くらいですね.
*5:c) = getchar(
*6:代入を受ける左辺値をカッコで囲むというのが不自然ですが,文法上,問題ありません.
*7:とはいえ,このプログラムでは,if文の前に出しても問題なく実行できます.入力の字数が少ないときは,まずwinnerとloserに意味のない値が代入されてから,「strlen(line) != 3」が真になるのでそこでおしまいです.とはいっても,プログラムを読む際の制御が歪みそうなので,採用しません.