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]; } }