「キミのコードが汚い理由」のコードが汚い理由
キミのコードが汚い理由 - ITmedia エンタープライズ
言いたいことはともかく、コードがひどすぎですよ。。この手は話はコードで説得してくれ、と言いたくなります。
とりあえず、自分のコードをさらします。たぶん自分ならこう書くでしょう。*1
class SetScorer { private int games[2] = {0, 0}; public static final int MINIMAL_DIFFERENCE = 2; public static final int MAX_SCORE = 7; public static final int MINIMAL_WIN = 6; void gameWon(int playerNo) { games[playerNo]++; } String getSetScore() { if (isGameOver()) { return "Player" + getWinner() + " won the game " + games[getWinner()] + "-" games[theOther(getWinner())]; } if (games[0] == games[1]) { return "Set is tied at " + games[0]; } return "Player" + getWinner() + " leads " + games[getWinner()] + "-" + games[theOther(getWinner())]; } private int theOther(int playerNo) { return 1 - playerNo; } private boolean isGameOver() { for (int i= 0; i < 2; i++) { if (games[i] >= MAX_SCORE) { return true; } if (games[i] >= MINIMAL_WIN && games[i] - games[theOther(i)] >= MINIMAL_DIFFERENCE) { return true; } } // not correct if tied private int getWinner() { int winner = (games[0] > games[1]) ? 0 : 1; } }
彼もいっているように、コードの美しさは主観でしか計れませんが、たぶん多くの人にとっては私のほうがわかりやすいと思います。私から見た、彼のコードが汚い理由。
- 名前の付け方が悪いです。人間が読むメッセージを返すメソッドの名前がgetSetScoreでは、徹夜でがんばっているプログラマの脳に負担をかけすぎです。
- 関数に詰め込みすぎです。このクラスの場合だと、テニスの試合が終わったか終わっていないかの判定が大事ですが、そこを関数や、メンバにくくりだしていないのは致命的です。これは、コーディングの対象をきちんと理解していないから大事な部分がくくりだせないのです。
わたしの経験から言わせてもらうと、彼のコードはごく平均的なプログラマのコードです。彼は、次のことに気をつけるべきです。
- 名前にもっと意識を持ちましょう。知り合いのエンジニアがいつも主張していることですが、「プログラマは名前をつけるのが仕事」です。名前は読みやすさに大きく影響します。
- 意味のあるコードのブロックは関数にして名前をつけましょう。小さい関数を作ることを恐れてはいけません。「わかりにくい条件」とわかっているなら、関数にして名前をつけましょう。
- よく考えて、ちゃんと理解してからコードを書きましょう。「われわれは一部のゲーム、セット、最終スコアにかなり複雑な条件があることに気付いた」と言っていますが、この程度で複雑なんて職業プログラマに失礼です。
- 全体として、コードの構造やアルゴリズムにとらわれているようですが、むしろセマンティクスに気を配るべきです。構造だけでは読みやすいコードは書けません。
ちなみに、読みやすさと同様にコードの品質として上げられるのがテストのしやすさですが、その視点で見てもメソッドを単体でテストできるので、私のほうがいいでしょうね。。
