昔作ったOOPなサンプルコードが出てきたからブログに残しておく
大昔に作ったOOPなコードの紹介記事を世に出していなかったので、恥ずかしいけど記録として公開しておきます。
今書けばもっと洗練できると思います(けど管理職になってから多忙で、その時間がないです…)。
オブジェクト指向プログラミングのサンプルコードご紹介
〜OOPらしく書くとコードはどう変わるか〜
題材
パスワードポリシー
Goal
以下の例を知って、OOPをやりたくなってもらう
- 複雑さを抑え込んだ変更に強いコード
- OOPらしく書くとコードが読みやすくなる
- 手続き型で作ると複雑さを抑え込めず読み難い
Story
具体例
要求仕様(ユーザストーリー)
- パスワード変更
- パスワードポリシーを満たせていたらパスワードを変更する
- 満たせていない場合はエラーを返す
パスワードポリシー
- パスワードの長さは8文字以上20文字以内
- 英数字大文字小文字を最低1回使用する
- ユーザ名と一致しない
- 現在のパスワードと一致しない
- 氏名(アルファベット)を含まない
- 大文字小文字は区別しない
- メールアドレスと一致しない
- 電話番号と一致しない
サンプルコード
https://github.com/kogayushi/ist-oop-refactoring-password-policy
Procedural
Production Code
public void changePassword(String id,String newPassword){ User user = this.userRepository.userFromId(UUID.fromString(id)); if (newPassword.length() < 8 || newPassword.length() > 20) { String msg = "inputted password violated password length policy"; throw new ViolatedPasswordPolicyException(msg); } // omitted the middle, coz it's too long. if (INCLUDING_UPPER_CASE_ALPHABET_AT_LEAST_ONE.matcher(newPassword).find() == false) { String msg = "inputted password violated character policy"; throw new ViolatedPasswordPolicyException(msg); } this.userRepository.updatePassword(user.getId(), new Password(newPassword)); }
特徴
- ポリシーを満たすかチェックするif文が10個以上並んでいる(例では略した)
- ポリシーにルールが増えるたびにif文が増える
- つまり、パスワード変更というユーザストーリーの中にパスワードポリシーの詳細が漏れている
OOP
Production Code
public void changePassword(ChangePasswordCommand command) { UUID id = command.getId(); User user = this.userRepository.userFromId(id); PasswordPolicy policy = this.policyFactory.generatePasswordPolicyFor(user); Password password = new Password(command.getNewPassword()); this.policy.validate(password); this.userRepository.updatePassword(id, password); }
特徴
- 分岐(if文等)がない
- パスワードポリシーの詳細をサービスクラスが知らない(知らなくて済む)
- ポリシーのみを切り離して単体テストが可能
サンプルコードの複雑さを観測してみる
- Cyclotic Complexityについては後述
Procedure/OOP | Cyclotic Complexity |
---|---|
Procedure | 13 |
OOP | 1 |
- 手続き型も13なら、悪くはない
- OOPは1と最小値
What's cyclomatic complexity
- コードの複雑さを表す指標
- if文などで分岐が増えるとポイントが1増える
- if文やswitchがなければcyclomatic complexityは1(最小値)
- 分岐がなくてもメソッドが2つ生えてたら最低で2
- 参考URL(不正確らしいがわかりやすいので紹介)
目安
循環的 複雑度 |
複雑さの状態 | バグ 混入確率 |
---|---|---|
10以下 | 非常に良い構造 | 25% |
30以上 | 構造的なリスクあり | 40% |
50以上 | テスト不可能 | 70% |
75以上 | いかなる変更も誤修正を生む | 98% |
サンプルコードの複雑さを観測してみる(再掲)
Procedure/OOP | Cyclotic Complexity |
---|---|
Procedure | 13 |
OOP | 1 |
- 現時点ではどちらも悪くはない
- OOPで書いたほうが指標で判断すれば優れていると言える
仕様変更 is coming !
その1:ユーザ名も変更可能にしたい
- パスワードと同じポリシーを適用したい
Procedural
修正ポイント
changePassword
をコピペしてchangeUsername
を作成changePassword
とchangeUsername
の重複を排除
Production Code
public void changeUsername(User user,String newUsername) { User user = this.userRepository.userFromId(UUID.fromString(id)); this.validateCommonPolicy(user, newPassword, "password"); this.userRepository.updatePassword(user.getId(), new Password(newPassword)); } private void validateCommonPolicy(User user, String newAuthenticationFactor, String name) { if (newAuthenticationFactor.length() < 8 || newAuthenticationFactor.length() > 20) { log.warn("inputted password violated password length policy"); throw new ViolatedPasswordPolicyException(msg); } // omitted, a lot. }
OOP
修正ポイント
PasswordPolicy
をコピペしてUsernamePolicy
を作成generatePasswordPolicyFor
をコピペしてgenerateUsernamePolicyFor
を作成changePassword
をコピペしてchangeUsername
を作成PasswordPolicy
とUsernamePoilcy
の重複を排除
Production Code
public void changeUsername(ChangeUsernameCommand command) { UUID id = command.getId(); User user = this.userRepository.userFromId(id); UsernamePolicy policy = this.policyFactory .generateUsernamePolicyFor(user); Username username = new Username(command.getNewUsername()); this.policy.validate(username); this.userRepository.updateUsername(id, username); }
その2:ユーザ名の文字長の制限は
4文字以上10文字以内にしたい
Procedure
修正ポイント
- 文字長チェックのif分をvalidateCommonPolicyからchangePasswordとchangeUsernameにコピー
- usernameの文字長下限のチェックを8から4に変更
Production Code
public void changeUsername(String id, String newUsername) { // omitted if (newUsername.length() < 4 || newUsername.length() > 20) { String msg = "inputted uername violated username length policy"; log.warn(msg); // it's warn just for testing. throw new ViolatedPolicyException(msg); } this.validateCommonPolicy(newUsername, user, "username"); this.userRepository.updateUsername(user.getId(), new Username(newUsername)); }
OOP
修正ポイント
- LengthPolicyをcommonPolicyから両方にコピペ
- usernameの方だけ文字長を8から4に変更
Production Code
public PasswordPolicy generatePasswordPolicyFor(User user) { Set<Policy> policies = generateCommonPolicy(user); policies.add(new LengthPolicy(8, 20)); return new PasswordPolicy(policies); } public UsernamePolicy generateUsernamePolicyFor(User user) { Set<Policy> policies = generateCommonPolicy(user); policies.add(new LengthPolicy(4, 20)); return new UsernamePolicy(policies); }
その3:ユーザ名はメールアドレスとの重複可
Procedure
修正ポイント
- メールアドレスとの一致チェックのif文をvalidateCommonPolicyからchangePasswordに移動
Production Code
public void changePassword(String id, String newPassword) { // omitted if (newPassword.length() < 8 || newPassword.length() > 20) { String msg = "inputted password violated password length policy"; log.warn(msg); // it's warn just for testing. throw new ViolatedPolicyException(msg); } if (newPassword.equals(user.getPerson().getContactInformation().getMailAddress().getValue())) { String msg = "inputted password violated not same with mail address policy"; log.warn(msg); throw new ViolatedPolicyException(msg); } // omitted, a lot. }
OOP
修正ポイント
- NotSameWithMailAddressPolicyをgenerateCommonPolicyから移動
Production Code
public PasswordPolicy generatePasswordPolicyFor(User user) { Set<Policy> policies = generateCommonPolicy(user); policies.add(new LengthPolicy(8, 20)); policies.add(new NotSameWithMailAddressPolicy(user.getPerson().getContactInformation().getMailAddress())); return new PasswordPolicy(policies); }
まとめ
手続き型を振り返ると
- 共通ロジックをメソッドにくくりだすことで重複が排除できた
- しかし、確かに重複は排除出来たが複雑度が減ったわけではない
- ユーザ名とパスワードのポリシーの違いが増えるたびに複雑度は増える
OOPらしく書くと何が違うか?
- ユーザ名を変更するメソッドが増えても複雑度は1しかあがらない
- パスワードポリシーのルールが変わったとしてもサービスクラス(ビジネスロジック)に影響がない
- 1つ1つの処理(今回はポリシー)に名前がつくので全体的に読みやすい
- サービスクラスがシンプル(分岐がない)になる
今回は利用側のクラスに着目したが…
- 利用される側であるPolicyやFactoryについても仕様変更に影響してcyclomatic complexityのポイントがあがることはない
- cyclomatic complexityはあくまで指標
- 肝心なのは、業務ルールがカプセル化されることにより利用側のクラスがその詳細を知らずに済むこと
- 業務の複雑さはなくせない。しかし、狭い範囲に閉じ込めることで変更に強くなる
例えば今回は
- ポリシーをオブジェクトにしたことで
- パスワードリセット機能など、パスワードを変更する他の機能でも利用できる(再利用性)
- メールアドレスのアカウント部と一致してはいけない、などのポリシーの変更に強い(変更容易性)