アンチパターン集

レイヤ違反は、書いた時点では動きます。 テストも通ります。 問題が表に出るのは、実装を差し替えたくなったとき、同じ処理をジョブから呼びたくなったとき、テーブル定義を変えたときです。

ここに挙げるのは、レビューで繰り返し指摘される十の実装です。 どれも一行から二行の違いでしかなく、書いた本人には違いが見えにくいという共通点があります。

エンドポイントから DAO を直接呼ぶ

// 避けたい実装
@Path("/api/foo/order/{orderId}")
@GET
public OrderEntity get(@Variable(name = "orderId") final String orderId) {
    final OrderDAO dao = DAOFactory.getTenantDatabaseDAO(OrderDAO.class);
    return dao.find(orderId);
}

// 望ましい実装
@Path("/api/foo/order/{orderId}")
@GET
public OrderResponse get(@Variable(name = "orderId") final String orderId) throws BusinessErrorException {
    try {
        return useCase.execute(orderId);
    } catch (final OrderAppException e) {
        throw new BusinessErrorException(e.getMessage(), e);
    }
}

プレゼンテーション層からインフラストラクチャ層を直接参照すると、あいだの二層が持っていた業務ルールが適用されません。 サービスが行う業務としての妥当性の判定も、ドメインモデルへの変換も、経由しなければ効きません。

エンティティを API のレスポンスに返す

// 避けたい実装
public OrderEntity findOrder(final String orderId) {
    return dao.find(orderId);
}

// 望ましい実装
public OrderResponse findOrder(final String orderId) throws OrderServiceException {
    final Order order = orderService.findByOrderId(orderId);
    return OrderResponse.fromDomainModel(order);
}

エンティティはテーブルの形をそのまま写したクラスです。 外に出せば、内部のカラム名と監査項目がクライアントに見えます。 それだけでなく、テーブル定義を変えた瞬間に API の互換性が壊れます。

リポジトリに業務ルールを書く

// 避けたい実装
public class StandardOrderRepository implements OrderRepository {

    @Override
    public void save(final Order order) throws RepositoryException {
        if (order.getAmount().compareTo(BigDecimal.ZERO) < 0) {
            throw new RepositoryException("金額が不正です");
        }
        // 永続化処理
    }
}

// 望ましい実装
public class StandardOrderService implements OrderService {

    @Override
    public void save(final Order order) throws OrderServiceException {
        if (order.getAmount().compareTo(BigDecimal.ZERO) < 0) {
            throw new OrderServiceException("金額は0以上である必要があります: amount=" + order.getAmount());
        }
        orderRepository.save(order);
    }
}

リポジトリが担うのはデータアクセスと、エンティティとドメインモデルの変換だけです。 業務ルールをここに書くと、リポジトリを差し替えたときにルールも一緒に消えます。

ドメイン層がインフラストラクチャ層を参照する

// 避けたい実装
package jp.co.example.foo.domain.service;

import jp.co.intra_mart.mirage.ext.dao.DAOFactory;
import jp.co.intra_mart.mirage.ext.session.SessionTemplate;

public class StandardOrderService {

    public Order findByOrderId(final String orderId) {
        final OrderDAO dao = DAOFactory.getTenantDatabaseDAO(OrderDAO.class);
        // ...
    }
}

// 望ましい実装
package jp.co.example.foo.infrastructure.service;

import jp.co.example.foo.domain.repository.OrderRepository;

public class StandardOrderService implements OrderService {

    private final OrderRepository orderRepository;
    // ...
}

レビューでは import 文だけを見れば済みます。 ドメイン層のクラスに jp.co.intra_mart.mirage.* が現れていれば、その時点で依存が外向きになっています。 実装クラスはインフラストラクチャ層に置き、ドメイン層にはインタフェースだけを残します。

例外を握り潰す

// 避けたい実装
try {
    orderRepository.save(order);
} catch (final RepositoryException e) {
    // 何もしない
}

// 望ましい実装
try {
    orderRepository.save(order);
} catch (final RepositoryException e) {
    LOGGER.error("Failed to save order: orderId=" + order.getOrderId(), e);
    throw new OrderServiceException("発注の保存に失敗しました", e);
}

握り潰すと、更新が一部だけ適用された状態のまま処理が続きます。 呼び出し側は成功したと解釈するため、データの不整合は次の処理か、翌日の集計で表に出ます。

トランザクション境界をエンドポイントに置く

// 避けたい実装
@Path("/api/foo/order")
@POST
public void create(@Body final OrderRequest request) {
    SessionTemplate.execute(new SessionCallback<Void, RuntimeException>() {
        @Override
        public Void execute(final Session session) {
            orderService.register(request);
            return null;
        }
    });
}

// 望ましい実装
@Override
public void register(final Order order) throws OrderServiceException {
    SessionTemplate.execute(s -> {
        try {
            orderRepository.save(order);
        } catch (final RepositoryException e) {
            throw new OrderServiceException("発注の登録に失敗しました: orderId=" + order.getOrderId(), e);
        }
        return null;
    });
}

境界を張るのは、サービスとリポジトリの実装です。 境界を外側に持ち出すと、SessionTemplate への依存がプレゼンテーション層まで広がります。 リポジトリをインタフェースで隠した意味がなくなり、同じ処理をジョブから呼ぶときに境界を書き直すことになります。

業務例外のメッセージを英語で書く

// 避けたい実装
throw new OrderServiceException("Order not found: id=" + orderId);

// 望ましい実装
throw new OrderServiceException("発注が見つかりません: orderId=" + orderId);

書き分けの基準は、読む相手が誰かにあります。 業務例外のメッセージは運用の担当者や利用者の目に触れるため、日本語で書き、原因の特定に必要な変数値を添えます。 開発者しか読まないログメッセージは、英語のままでかまいません。

ファクトリを使わずに直接 new する

// 避けたい実装
public class GetOrderUseCase {

    private final OrderService orderService = new StandardOrderService();
}

// 望ましい実装
public class GetOrderUseCase {

    private final OrderService orderService;

    public GetOrderUseCase() {
        try {
            this.orderService = OrderServiceFactory.getInstance().getOrderService();
        } catch (final OrderServiceException e) {
            throw new RuntimeException("OrderService の初期化に失敗しました", e);
        }
    }
}

new で書くと、呼び出し側が既定の実装に固定されます。 ServiceLoaderUtil による差し替えも、テストでのモック注入も効かなくなります。

入力検証のエラーをログに出力する

// 避けたい実装
final List<String> errors = GetOrderValidator.validate(orderId);
if (!errors.isEmpty()) {
    LOGGER.error("Validation failed: " + errors);
    throw new ValidationException(errors);
}

// 望ましい実装
final List<String> errors = GetOrderValidator.validate(orderId);
if (!errors.isEmpty()) {
    throw new ValidationException(errors);
}

入力の不備は利用者の操作の結果であり、システムの障害ではありません。 ERROR として記録すると、対処すべき障害がログの中に埋もれます。 認可エラーも同じ理由で記録しません。

監査項目を手で設定する

// 避けたい実装
entity.createUserCd = "admin";
entity.createDate = new Timestamp(System.currentTimeMillis());
entity.recordUserCd = "admin";
entity.recordDate = new Timestamp(System.currentTimeMillis());
dao.insert(entity);

// 望ましい実装
dao.insert(entity);

AbstractDAO の insert は監査項目の四フィールドを、update は更新者と更新日時の二フィールドを自動で設定します。 手で設定すると、ログインユーザとは無関係の値が記録され、更新履歴を追えなくなります。 監査項目の扱いはエンティティと DAO の作成で扱っています。

関連ドキュメント

• 全体アーキテクチャ：レイヤの責務と依存の向き
• インフラストラクチャ層の実装ルール：トランザクション境界と例外の変換
• プレゼンテーション層の実装ルール：エンドポイントの責務と入力検証