1 クラス 1 機能、1 メソッド 1 処理の徹底

どういう事？

名前の通り、クラス設計において、１つの機能は１クラスとして用意します。
また、１機能の実現もまた手続きがありますので、それら手続きも基本的にクラス、メソッドとします。

さらに、手続きの中でもループして書き換えるとか、なんらかの値に変更するなどの「その操作そのものは機能と言えないが、しかし一つの手順ではある」というコードをメソッド化します。

要するにあらゆる手続きは全てメソッド化し、分離可能なら処理毎にクラス化し、クラス単位の処理内容を徹底して単一化します。

場合によってはクラス内にメソッドは 1 つ、行数が数行という場合も発生しますが、それでもやります。

なぜ？

病的なまでに処理を分離するということは、クラス/メソッドの役割がそれだけシンプルになる。
これにクラス名、メソッド名を適切に設定すれば、修正が必要な際にも対象のクラスを即座に発見することができるようになります。
また、処理が十分に分離されているということは、処理は十分に単純であるということ。

単純な処理ほど 再利用が行いやすくなります 。
KISS原則も必然的に守れますね。

どうする？

これはコーディング時にメソッド化すべき物は何かを常に考えます。
例えば、ログイン処理なんかはこんな感じでしょう。

コード自体はサンプルコードです。コピペしても多分コンパイルできないかと

最初に、メソッドと処理内容をざっくり考えます。

public HTTPResponse getProfile(final HTTPRequest request) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

では実装を開始してみましょう。

@Autowierd
private AuthorityRepository authorityRepository;

public HTTPResponse getProfile(final HTTPRequest request, final HTTPSession session) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    Optional<Long> loginAccountId =
        session.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong);
    Optional<Authority> authority =
        loginAccountId.flatMap(id -> authorityRepository.getOneByAccountId(id));

    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

でもこれは要するに手続きで、目的はセッションをキーに権限情報を取得する事にありますから、メソッド化した方が余計な変数が増えずに済みます。

public HTTPResponse getProfile(final HTTPRequest request, final HTTPSession session) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    Optional<Authority> authority = this.getLoginAuthority(session);

    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

private Optional<Authority> getLoginAuthority(final HTTPSession session) {
    Optional<Long> loginAccountId =
        session.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong);
    return loginAccountId.flatMap(id -> authorityRepository.getOneByAccountId(id));
}

セッションからアカウントIDを取得する事は、およそ共通の処理でしょう。
ならば utils サブパッケージを作って分離しておくべきでしょう。

package controllers.utils;

public class SessionManagement {
  public static Optional<Long> readLoginAccountId(final HTTPSession s) {
    return s.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong)
  }

追加で

@Autowierd
private AuthorityRepository authorityRepository;

public Optional<Authority> getLoginAuthority(final HTTPSession session) {
    return readLoginAccountId(session)
        .flatMap(authorityRepository::findOneById);
}

こうした分離を繰り返すと、いつしか処理が本来の「getProfile（プロフィール取得）」から外れて、個々の小さな処理となります。
これを適切なクラスに分離していくことで、再利用性の高いクラスが出来ていき、同じような事を繰り返す事がなくなっていきます。

蛇足ですが、見ての通り、汎用的な処理とは即ち極小の、業務ロジックに直接関係しない個々の処理である事がわかります。
これは全体を眺めただけで見通すのは難しい。

設計はトップダウンとボトムアップ - 謎言語使いの徒然にも繋がる話となります。

小さく区切って整理する。これが保守性を上げる一助となります。

私なら、getProfile は私ならこんな書き方をさせますね。

public Response getProfile(final HTTPRequest request, final HTTPSession session) {

    // 権限を取得し、取れないようなら、未ログインとして蹴ります
    Optional<Authority> authority = sessionUtils.getLoginAuthority(session);
    if (authority.isEmpty()) return redirect(request, Constants.LOGIN_URL);

    // リクエストから取得するプロフィールの ID を取得します。
    return this.readProfileId(request)

        // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
        .filter(targetId -> this.profiles.isAccessibleProfile(targetId, authority.get()))

        // 取得した ID をキーに、対象のアカウントを DB から取得します。
        .flatMap(targetId -> this.profiles.getByAccountId(targetId))

        // アクセス取得できる場合は、表示用の画面を返します。
        .map(prof -> renderProfilePage(prof))

        // アクセスができなければエラー画面を返します。
        .orElse(redirect(request, Constants.HOME).with(
            Flash.error(Messages.get(Constants.ERROR_FORBIDDEN))
        ));
}