グループワークに向けて(4)
今回の目的
前回の内容を自分が以前書いたコードを使用して実践します。また、「リーダブルコード」の残されている部分に触れていきます。
実践しよう!
変更前
今回使用するコードは、僕が以前書いたlife gameのコードです。当時、life gemeの仕組みがよくわかっていなかったので、セルが誕生か死亡するときにエフェクトをつけています。確か3,4ヶ月ほど前のコードなのでどの様に書いたかは完全に忘れています。言語はprocessingの2系で書いています。
int num = 400; int side = 2; int [][] pc = new int [400][400]; int [][] cell = new int [400][400]; int [][] nc = new int [400][400]; int c = 0; void setup() { frameRate(500); fill(100, 255, 100); noStroke(); size(800, 800); for (int i = 0; i < 400; i++) { for (int j = 0; j < 400; j++) { cell[i][j] = (int)random(6); if (cell[i][j] > 1) { cell[i][j] = 0; } } } } void draw() { background(0); for (int i = 0; i < num; i++) { for (int j = 0; j < num; j++) { if (cell[i][j] == 1) { if (frameCount%50 != 0) { if (pc[i][j] == 0) { fill(0, 5*(frameCount%50)+5, 0); rect(i*side, j*side, side, side); } else { fill(0, 255, 0); rect(i*side, j*side, side, side); } } } else if (cell[i][j] == 0) { if (frameCount%50 != 0) { if (pc[i][j] == 1) { fill(0, 255-5*(frameCount%50), 0); rect(i*side, j*side, side, side); } } } } } if (frameCount%50 == 0) { for (int i = 0; i < num; i++) { for (int j = 0; j < num; j++) { if (cell[i][j] != 0) { fill(0,255,0); rect(i*side, j*side, side, side); } count(i, j); judge(i, j); } } } if (frameCount%50 == 0) { for (int i = 0; i < num; i++) { for (int j = 0; j < num; j++) { pc[i][j] = cell[i][j]; cell[i][j] = nc[i][j]; } } } } int count(int x, int y) { c = 0; if (cell[(x+399)%400][(y+399)%400] >= 1) c++; if (cell[x%400][(y+399)%400] >= 1) c++; if (cell[(x+1)%400][(y+399)%400] >= 1) c++; if (cell[(x+399)%400][y%400] >= 1) c++; if (cell[(x+1)%400][y%400] >= 1) c++; if (cell[(x+399)%400][(y+1)%400] >= 1) c++; if (cell[x%400][(y+1)%400] >= 1) c++; if (cell[(x+1)%400][(y+1)%400] >= 1) c++; return c; } void judge(int x, int y) { if (cell[x][y] == 0) { nc[x][y] = (c==3 ? 1 : 0); } else if (cell[x][y] == 2) { nc[x][y] = 1; } else if (cell[x][y] == 1) { if(c==2||c==3){ nc[x][y] = 1; }else{ nc[x][y] = 0; } } }
ため息が出るほど汚いコードですね...。ではここからまずは表面的に変えていきましょう。
第一段階
まずはグループワークに向けて(2)で取り扱った『表面上の改善』を行っていきましょう。
static final int NUM = 600; static final int SIDE = 2; static final int UNIT_PERIOD = 50; int [][] previousCell = new int [NUM][NUM]; int [][] currentCell = new int [NUM][NUM]; int [][] nextCell = new int [NUM][NUM]; int c = 0; void setup() { size(NUM*SIDE, NUM*SIDE); frameRate(500); fill(100, 255, 100); noStroke(); // セルを1/6の確率で生きている状態 // その他は死んでいる状態にする for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { currentCell[i][j] = (int)random(6); if (currentCell[i][j] > 1) { currentCell[i][j] = 0; } } } } void draw() { background(0); for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { // セルの生死の移り変わりを表現 if (currentCell[i][j] == 1) { if (frameCount%UNIT_PERIOD != 0) { if (previousCell[i][j] == 0) { fill(0, 5*(frameCount%UNIT_PERIOD)+5, 0); rect(i*SIDE, j*SIDE, SIDE, SIDE); } else { fill(0, 255, 0); rect(i*SIDE, j*SIDE, SIDE, SIDE); } } } else if (currentCell[i][j] == 0) { if (frameCount%UNIT_PERIOD != 0) { if (previousCell[i][j] == 1) { fill(0, 255-5*(frameCount%UNIT_PERIOD), 0); rect(i*SIDE, j*SIDE, SIDE, SIDE); } } } } } // 次世代を計算し、代入し直す if (frameCount%UNIT_PERIOD == 0) { for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { if (currentCell[i][j] != 0) { fill(0, 255, 0); rect(i*SIDE, j*SIDE, SIDE, SIDE); } countAroundLiveCell(i, j); decideNextGeneration(i, j); } } for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { previousCell[i][j] = currentCell[i][j]; currentCell[i][j] = nextCell[i][j]; } } } } // 2次元トーラスを再現 int countAroundLiveCell(int x, int y) { c = 0; if (currentCell[(x+NUM-1)%NUM][(y+NUM-1)%NUM] >= 1) c++; if (currentCell[x%NUM][(y+NUM+1)%NUM] >= 1) c++; if (currentCell[(x+1)%NUM][(y+NUM-1)%NUM] >= 1) c++; if (currentCell[(x+NUM-1)%NUM][y%NUM] >= 1) c++; if (currentCell[(x+1)%NUM][y%NUM] >= 1) c++; if (currentCell[(x+NUM-1)%NUM][(y+1)%NUM] >= 1) c++; if (currentCell[x%NUM][(y+1)%NUM] >= 1) c++; if (currentCell[(x+1)%NUM][(y+1)%NUM] >= 1) c++; return c; } // (x, y)にあるセルの次の世代での生死をnextCellに代入 // 死なら0、生なら1、生存なら2(?) void decideNextGeneration(int x, int y) { if (currentCell[x][y] == 0) { nextCell[x][y] = (c==3 ? 1 : 0); } else if (currentCell[x][y] == 2) { nextCell[x][y] = 1; } else if (currentCell[x][y] == 1) { if (c==2||c==3) { nextCell[x][y] = 1; } else { nextCell[x][y] = 0; } } }
変数名や関数名をわかりやすくしてコメント入れてレイアウトを気にして...という感じに改善してみました。コードはほとんど読まずに変更したので本当に表面上の改善でしかありませんが、前のコードよりは読みやすくなっているのかな?という感じですね。
第二段階
次にグループワークに向けて(3)の『ループとロジックの単純化』を元に問題点を探しましょう。
- 不要なグローバル変数
c
が存在する draw
関数内のネストが深いcountAroundLiveCell(x, y)
関数がわかりにくい(汚い)- 三項演算子が使われているがわかりやすさはどうか
- 描画する際の色がわかりずらい
- セルの状態2は必要なのか?
- 定数に付いてる
static
も不要ではないか
これらの問題点を元に、第一段階のコードを変更していきましょう。
final int NUM = 200; final int SIDE = 5; final int UNIT_PERIOD = 50; final color GREEN = color(0, 255, 0); int [][] previousCell = new int [NUM][NUM]; int [][] currentCell = new int [NUM][NUM]; int [][] nextCell = new int [NUM][NUM]; void setup() { size(NUM*SIDE, NUM*SIDE); // 透明度の範囲を0〜(UNIT_PERIOD-1)に変更 colorMode(RGB, 256, 256, 256, UNIT_PERIOD); frameRate(500); noStroke(); // セルを1/6の確率で生きている状態 // その他は死んでいる状態にする for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { currentCell[i][j] = (int)random(6); if (currentCell[i][j] > 1) { currentCell[i][j] = 0; } } } } void draw() { background(0); // 次世代を計算し、代入し直す if (frameCount%UNIT_PERIOD == 0) { for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { countAroundLiveCell(i, j); decideNextGeneration(i, j); if (currentCell[i][j] == 0) continue; fill(GREEN, UNIT_PERIOD); rect(i*SIDE, j*SIDE, SIDE, SIDE); } } for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { previousCell[i][j] = currentCell[i][j]; currentCell[i][j] = nextCell[i][j]; } } } else { for (int i = 0; i < NUM; i++) { for (int j = 0; j < NUM; j++) { // セルの生死の移り変わりを表現 if (currentCell[i][j] == 0) { if (previousCell[i][j] == 0) continue; fill(GREEN, UNIT_PERIOD-(frameCount%UNIT_PERIOD)); } else { if (previousCell[i][j] == 0) fill(GREEN, frameCount%UNIT_PERIOD); else fill(GREEN, UNIT_PERIOD); } rect(i*SIDE, j*SIDE, SIDE, SIDE); } } } } // 2次元トーラスを再現 int countAroundLiveCell(int x, int y) { int c = 0; int left = (x+NUM-1)%NUM; int top = (y+NUM-1)%NUM; int center = x%NUM; int middle = y%NUM; int right = (x+1)%NUM; int bottom = (y+1)%NUM; if (currentCell[left] [top] == 1) c++; if (currentCell[center] [top] == 1) c++; if (currentCell[right] [top] == 1) c++; if (currentCell[left] [middle] == 1) c++; if (currentCell[right] [middle] == 1) c++; if (currentCell[left] [bottom] == 1) c++; if (currentCell[center] [bottom] == 1) c++; if (currentCell[right] [bottom] == 1) c++; return c; } // (x, y)にあるセルの次の世代での生死をnextCellに代入 // 死なら0、生なら1 void decideNextGeneration(int x, int y) { int count = countAroundLiveCell(x, y); if (currentCell[x][y] == 0) nextCell[x][y] = (count==3 ? 1 : 0); else nextCell[x][y] = (count==2||count==3 ? 1 : 0); }
c
をcountAroundLiveCell
関数内のローカル変数にし、独立させた- 同じ内容を括りだしたり、順序を入れ替えてわかりやすく変更
- 中身を整理して変数に置き換えることでわかりやすくした
- 三項演算子のほうがわかりやすい、逆に
if
文のが分かりづらかったので変更 GREEN
という定数に置き換え、透明度で濃淡を変えることに- 不要だったので排除した
- 上に同じく、排除した
特に3番の内容変更を見てください。実は第1段階のコードではcountAroundLiveCell
内の符号を間違えていたため、プログラムが思った通りに動いていませんでした。(関数内2行目[(y+NUM+1)%NUM] ⇒ [(y+NUM-1)%NUM]
)それを変数に置き換えて分割することで、わかりやすくなった上にバグもなくせました。素晴らしい変更ですね!
最終段階
最後は第Ⅲ部の『コードの再構成』に準じて、コードを考え直してみましょう。考え直すことをリストアップしてみました。
- 無関係の下位問題がたくさんある
- セルをインスタンスにしたほうがわかりやすい
- 本当に読みやすいコードになっているのか?
以上を踏まえて、コードを再構成してみました。まずはCell
クラスから!
class Cell { // row : 横の行 // column : 縦の列 // 0からスタート final int row; final int column; boolean wasArive = false; boolean isArive; Cell(int _row, int _column) { this.row = _row; this.column = _column; // セルを1/6の確率で生にする this.isArive = int(random(10))%7==0; this.wasArive = this.isArive; } int cellAlpha() { if (frameCount%UNIT_PERIOD==0) return UNIT_PERIOD-1; if (this.isArive == this.wasArive) return UNIT_PERIOD-1; else if (this.isArive == true) return frameCount%UNIT_PERIOD; // 誕生 else return UNIT_PERIOD-(frameCount%UNIT_PERIOD); // 死亡 } void draw() { if (!this.isArive && !this.wasArive) return; if (frameCount%UNIT_PERIOD==0 && !wasArive) return; fill(GREEN, this.cellAlpha()); rect(this.row*SIDE, this.column*SIDE, SIDE, SIDE); } // 2次元トーラスを再現 int countAroundAriveCell() { int count = 0; int left = (this.column+NUM-1)%NUM; int top = (this.row +NUM-1)%NUM; int center = this.column%NUM; int middle = this.row %NUM; int right = (this.column+1)%NUM; int bottom = (this.row +1)%NUM; if (cells [top] [left] .wasArive) count++; if (cells [top] [center] .wasArive) count++; if (cells [top] [right] .wasArive) count++; if (cells [middle] [left] .wasArive) count++; if (cells [middle] [right] .wasArive) count++; if (cells [bottom] [left] .wasArive) count++; if (cells [bottom] [center] .wasArive) count++; if (cells [bottom] [right] .wasArive) count++; return count; } boolean suggestNextGeneration() { int count = this.countAroundAriveCell(); if (this.wasArive) return count==2||count==3; else return count==3; } void shiftNextGeneration() { boolean willArive = this.suggestNextGeneration(); this.wasArive = this.isArive; this.isArive = willArive; } }
次にメインのコードです。
final int NUM = 400; final int SIDE = 2; final int UNIT_PERIOD = 30; final color GREEN = color(0, 255, 0); Cell cells [/* row */] [/* column */] = new Cell [NUM][NUM]; void setup() { size(NUM*SIDE, NUM*SIDE); frameRate(10); noStroke(); // 透明度の範囲を変更 // 透明 0〜(UNIT_PERIOD-1) 不透明 colorMode(RGB, 256, 256, 256, UNIT_PERIOD); for (int _row = 0; _row < NUM; _row++) { for (int _column = 0; _column < NUM; _column++) { cells [_row] [_column] = new Cell(_row, _column); } } } void draw() { background(0); if (frameCount%UNIT_PERIOD == 0) { for (int _row = 0; _row < NUM; _row++) { for (int _column = 0; _column < NUM; _column++) { cells [_row] [_column].shiftNextGeneration(); } } } for (int _row = 0; _row < NUM; _row++) { for (int _column = 0; _column < NUM; _column++) { cells [_row] [_column].draw(); } } }
無関係の下位問題を抽出し、クラスに置き換えました。コードのコメントも工夫を凝らしてわかりやすくなるようにしてみました。
が!まだまだ問題が山積みです...。
- 無関係の下位問題を抽出しきれていない
- 一度に一つのことだけを行えていない
- 各関数などが完全に独立しきれていない
- (これは完全に僕の実力不足ですが、life gameが正常に動作しているかわかりません...)
と、このように経験が浅いためか上手にわかりやすいコードを書けませんでした。 やはり何度も意識的にコードを書いていくことが重要になるのでしょう。 日々精進して行きましょう!
『リーダブルコード』の第Ⅳ部
今回参考にしたのは『リーダブルコード』の第Ⅰ〜Ⅲ部でした。 しかし、この本には第Ⅳ部があり、そこではより具体的な例を挙げて実践的な内容に触れています。 また、付録にはオススメの本が載っていたりと、まだまだ紹介しきれていない部分がたくさんあります。 なにより僕にはこの本の内容を全て伝えきれるだけの説明力がありませんでした。 なので皆さんには実際にこの本を手にとって読んでいただきたいです。 内容としてはあまり難しいものではなく、特にプログラミング初心者の方や共同作業を目前に控えた方に読んでいただきたい一冊です。
リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)
- 作者: Dustin Boswell,Trevor Foucher,須藤功平,角征典
- 出版社/メーカー: オライリージャパン
- 発売日: 2012/06/23
- メディア: 単行本(ソフトカバー)
- 購入: 68人 クリック: 1,802回
- この商品を含むブログ (135件) を見る
まとめ
実際に読みやすいコードを書こうとするのはとても大変で、頭をよく使う作業でした。 「読み手はここがわかりづらいんじゃないか」、「ここはこう書いたほうがいいんじゃないか」と試行錯誤しながらコードを改善していました。 まるで国語の文章をを書いているかのようでした笑。 しかしコードが読みやすくなったかと言われると、素直に頷けるものではありません。 今後も『より良いコード』を目指して読み手を意識しながらコードを書いていきます! 次回は総まとめとして、今までの内容をすべて振り返っていきます。