KISSの原則とは
Keep It Simple, Stupid.
Keep It Short and Simple.
シンプルにしておけ、愚か者よ。
簡潔かつ単純にしておけ。
この言葉はソフトウェア開発の原則としても有名で、ケリー・ジョンソンというかた提唱れました。
簡単に言うと、ソフトウェア設計をより単純にすることが成功につながるといったものです。
プログラムは要件が増えれば増えるほど仕様がより複雑になり、作られるものはシンプルでなくなっていきます。
あれもやるかもしれない、これもやるかもしれない、技術的には可能だからと考えすぎると機能がより複雑になりユーザー使えないものとなってしまいます。
複雑なものほど、それを解決するプログラムの書き方によって、メンテナンス性やパフォーマンスも大きく変わってきます。
どれだけ物事をシンプルに考えられるか、それがこの本質かと思います。
複雑に考えるとは何か
イベントのスケジュールを立てるシステムがあったとします。
ある会場ではイベントの開催時間が重複してはいけないため、スケジュールの重複チェックを行う必要があります。そのため元々登録されているスケジュールと重複があるかをチェックするメソッドを作ろうと思います。
スケジュールの重複チェックを考える
まずはプログラムを書く前にスケジュールの重複パターンについて考えてみます。
境界値も含めて考えた場合以下のようなパターンが考えられます。
※ このパターンは単体テストを作成する時にも使えます
重複 | 9時 | 10時 | 11時 | 12時 | 13時 | 14時 | 15時 | 16時 | 17時 | 18時 | |
---|---|---|---|---|---|---|---|---|---|---|---|
登録済 | − | ■ | ■ | ■ | ■ | ||||||
パターン1 | × | ■ | ■ | ||||||||
パターン2 | × | ■ | ■ | ||||||||
パターン3 | ○ | ■ | ■ | ||||||||
パターン4 | ○ | ■ | ■ | ||||||||
パターン5 | ○ | ■ | ■ | ||||||||
パターン6 | ○ | ■ | ■ | ||||||||
パターン7 | ○ | ■ | ■ | ||||||||
パターン8 | × | ■ | ■ | ||||||||
パターン9 | × | ■ | ■ | ||||||||
パターン10 | ○ | ■ | ■ | ■ | ■ | ||||||
パターン11 | ○ | ■ | ■ | ■ | ■ | ■ | ■ |
マトリックスをそのままプログラムで組んで見る
こんなプログラムを書く人はいないと思いますが、マトリックス通りにプログラムを書くとこのようなプログラムになります。
とても複雑でわかりにくいですね。
public class ScheduleChecker {
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeFrom)) {
// パターン1
return false;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeFrom)) {
// パターン2
return false;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.equals(reservedTimeFrom)) {
// パターン3
return true;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン4
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン5
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeFrom.isBefore(reservedTimeTo) && targetTimeTo.equals(reservedTimeTo)) {
// パターン6
return true;
} else if (targetTimeFrom.equals(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン7
return true;
} else if (targetTimeFrom.isAfter(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン8
return false;
} else if (targetTimeFrom.isAfter(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン9
return false;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.equals(reservedTimeTo)) {
// パターン10
return true;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン11
return true;
}
return false;
}
}
単体テストを書いてみる
上記のパターンをチェックするために単体テストを書いてみます。
プログラムをシンプルに書き直すために単体テストは必須です。
単体テストを書くことでプログラムの正しさを証明でき、シンプルにするためのリファクタリングを可能とします。
private static final LocalDateTime reservedTimeFrom = LocalDateTime.of(2020, 12, 24, 12, 0);
private static final LocalDateTime reservedTimeTo = LocalDateTime.of(2020, 12, 24, 15, 0);
@Test
public void pattern1() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 9, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 10, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(false));
}
@Test
public void pattern2() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 10, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 11, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(false));
}
@Test
public void pattern3() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 11, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 12, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern4() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 12, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 13, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern5() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 13, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 14, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern6() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 14, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 15, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern7() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 15, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 16, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern8() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 16, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 17, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(false));
}
@Test
public void pattern9() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 17, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 18, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(false));
}
@Test
public void pattern10() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 12, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 15, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
@Test
public void pattern11() {
final LocalDateTime targetTimeFrom = LocalDateTime.of(2020, 12, 24, 11, 0);
final LocalDateTime targetTimeTo = LocalDateTime.of(2020, 12, 24, 16, 0);
assertThat(ScheduleChecker.conflict(reservedTimeFrom, reservedTimeTo, targetTimeFrom, targetTimeTo), is(true));
}
シンプルに考えるとは何か
このようなプログラムを書いてしまうと、しばらく時間がたった時に他のプログラマーだけでなく本人ですらメンテナンスが大変になります。
ではプログラムをシンプルにすることを考えてみましょう。
プログラムの重複をチェックする
プログラムを見ると「パターン1」と「パターン2」、「パターン8」と「パターン9」のプログラムが重複しています。
こちらは余分なコードになるので統合していきます。
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeFrom)) {
// パターン1, 2
return false;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.equals(reservedTimeFrom)) {
// パターン3
return true;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン4
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン5
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeFrom.isBefore(reservedTimeTo) && targetTimeTo.equals(reservedTimeTo)) {
// パターン6
return true;
} else if (targetTimeFrom.equals(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン7
return true;
} else if (targetTimeFrom.isAfter(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン8, 9
return false;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.equals(reservedTimeTo)) {
// パターン10
return true;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン11
return true;
}
return false;
}
真偽状態のチェック
真偽の状態をチェックする場合、基本的に真(true) 偽(false)どちらを中心にチェックをかけるかを検討します。
今回のプログラムでは最終的に全てのチェックが通らなかった場合に false を返しています。
そのためプログラムが true の場合のみチェックを行えばいいことがわかります。
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.equals(reservedTimeFrom)) {
// パターン3
return true;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン4
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン5
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeFrom.isBefore(reservedTimeTo) && targetTimeTo.equals(reservedTimeTo)) {
// パターン6
return true;
} else if (targetTimeFrom.equals(reservedTimeTo) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン7
return true;
} else if (targetTimeFrom.equals(reservedTimeFrom) && targetTimeTo.equals(reservedTimeTo)) {
// パターン10
return true;
} else if (targetTimeFrom.isBefore(reservedTimeFrom) && targetTimeTo.isAfter(reservedTimeTo)) {
// パターン11
return true;
}
// パターン1, 2, 8, 9
return false;
}
境界値を考える
境界値の部分を検討するとよりシンプルに処理を書き換えることができます。
境界値と条件を考慮して以下のパターンを統合します。
- パターン3、パターン4
- パターン6、パターン7
- パターン10、パターン11
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
if (targetTimeFrom.compareTo(reservedTimeFrom) <= 0 && targetTimeTo.compareTo(reservedTimeFrom) >= 0 && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン3, 4
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeTo.isBefore(reservedTimeTo)) {
// パターン5
return true;
} else if (targetTimeFrom.isAfter(reservedTimeFrom) && targetTimeFrom.compareTo(reservedTimeTo) <= 0 && targetTimeTo.compareTo(reservedTimeTo) >= 0) {
// パターン6, 7
return true;
} else if (targetTimeFrom.compareTo(reservedTimeFrom) <= 0 && targetTimeTo.compareTo(reservedTimeTo) >= 0) {
// パターン10, 11
return true;
}
// パターン1, 2, 8, 9
return false;
}
シンプルに考える
ここまででも最初のプログラムに比べれば大分シンプルにまとまってきました。
ただ、チェックする際にそれぞれの時間の FROM、TO 同士を比べるこのやり方はまだ改善の余地があります。
そこで最初のマトリックスをしっかり見直してみます。
よく見てみると以下の条件の場合に重複があることがわかります。
- 予約しようとしているFROMが、登録済みのTO以前である
- 予約しようとしているTOが、登録済みのFROM移行である
実はこの条件だけでこの仕様を満たすことができます。
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
if (targetTimeFrom.compareTo(reservedTimeTo) <= 0 && targetTimeTo.compareTo(reservedTimeFrom) >= 0) {
// パターン3, 4, 5, 6, 7, 10, 11
return true;
}
// パターン1, 2, 8, 9
return false;
}
ここまでは短縮する必要はありませんが、基本的に真偽値を返すメソッドのため以下のように書くこともできる。
public static boolean conflict(
LocalDateTime reservedTimeFrom, LocalDateTime reservedTimeTo,
LocalDateTime targetTimeFrom, LocalDateTime targetTimeTo) {
return targetTimeFrom.compareTo(reservedTimeTo) <= 0 && targetTimeTo.compareTo(reservedTimeFrom) >= 0;
}
まとめ
ここまで見ていくとわかりますが、複雑にものごとを考えてしまうとそれに比例してプログラムも複雑になり、可読性、メンテナンス性、パフォーマンスも落ちていくことになります。
シンプルこそシステムの究極の形なのかもしれません。
シンプルに考えることは究極に考えると本質を見抜く行為になります。
仕様・設計・プログラムを考え作っている際に、なんか複雑だなと思ったら一度俯瞰してものを見てみることが重要かと思います。