kiyo_hikoのブログ

メモ+日記?

「キミのコードが汚い理由」という記事を読んだ

The Rational Edge:キミのコードが汚い理由 - ITmedia エンタープライズ


糞コードのサンプルと改善案も載ってるけど、改善案みたいに饒舌なトークンはめんどくさくて苦手。これってMECEで入ってきた条件に従ってStringを返すだけっぽいから条件演算子のテーブル使えば楽勝そう。

public class SetScorer {
	private int[] games = {0, 0};
	public void gameWon(int i) {
		games[i-1]++;
	}
	/**
	 * スコアルールについてはこのへんにJavadocの一部としてまとめて書く
	 */
	public String getSetScore() {
		int p1 = games[0], p2 = games[1];
		String p12 = p1 + " - " + p2;
		String p21 = p2 + " - " + p1;
		return  p1 == 7              ? "Player1 wins the set "      + p12
		      : p2 == 7              ? "Player2 wins the set "      + p21
		      : (p1 == 6 && p2 == 6) ? "Set is tied at 6 games"
		      : (p1 == 6 && p2 <  5) ? "Player1 wins the set "      + p12
		      : (p1 == 6 && p2 == 5) ? "Player1 leads "             + p12
		      : (p2 == 6 && p1 <  5) ? "Player2 wins the set "      + p21
		      : (p2 == 6 && p1 == 5) ? "Player2 leads "             + p21
		      : (p1 <  6 && p2 <  6 && p1 > p2) ? "Player1 leads "  + p12
		      : (p1 <  6 && p2 <  6 && p2 > p1) ? "Player2 leads "  + p21
		      :                                   "Set is tied at " + p1
	}
}

めんどくさいので、

  • 真偽値の算出パターンはそのまま(多分もっと分岐を減らせる)
  • 想像力を刺激するような新しくて長い変数名は入れてない
  • 勝ち点を定数に切り出してない
  • StringBuilder使ってない
  • クラスのAPIそのまま
  • 入れ子も省略

もとのサンプルが画像だったのでテキストにした。

public class SetScorer {
	private int[] games = {0, 0};
	public void gameWon(int i) {
		games[i-1]++;
	}
	public String getSetScore() {
		if (games[0] < 6 && games[1] < 6) {
			if (games[0] > games[1]) {
				return "Player1 leads " + games[0] + " - " + games[1];
			} else if (games[1] > games[0]) {
				return "Player2 leads " + games[1] + " - " + games[0];
			} else {
				return "Set is tied at " + games[1];
			}
		}
		if (games[0] == 6 && games[1] < 5) {
			return "Player1 wins the set " + game[0] + " - " + games[1];
		}
		if (games[1] == 6 && games[0] < 5) {
			return "Player2 wins the set " + game[1] + " - " + games[0];
		}
		if (games[0] == 6 && games[1] == 5) {
			return "Player1 leads 6 - 5";
		}
		if (games[1] == 6 && games[0] == 5) {
			return "Player2 leads 6 - 5";
		}
		if (games[0] == 6 && games[1] == 6) {
			return "Set is tied at 6 games";
		}
		if (games[0] == 7) {
			return "Player1 wins the set 7 - " + games[1];
		}
		return "Player2 wins the set 7 - " + games[0];
	}
}