Refactoring a Settlement Calculation Method: Identifying Code Smells and Applying Improvements

The original

private OrderShoudSettlementAmount getOrderShoudSettlementAmount(OrderDTO orderMain, List<SettlementDetail> details)

method assembles an OrderShoudSettlementAmount object by manually accumulating fee amounts for each expense type. It calculates two values: settlementMoney (the net amount) and goodCommissionMoney (the commission).

When a new fee type was added, the method was extended with additional variables ( feeMoney34012, feeMoney34013) and extra if branches, making the code longer and harder to maintain.

Applying Martin Fowler's catalog of code smells, two major issues were identified:

Duplicated Code : The for loop contains repetitive if blocks for each fee type, requiring manual updates whenever a new fee is introduced.

Divergent Change : Multiple unrelated modifications are needed in the same method whenever business rules change, violating the principle of localized change.

Beyond these smells, the method mixes object assembly with business calculations, violating object‑oriented design principles.

To address the problems, the method was refactored. The new implementation first aggregates fee amounts into a Map<Integer, Long> expenseTypeToFeeMoneyMap. Then two dedicated enum‑based calculators compute the settlement and commission values:

private OrderShoudSettlementAmount getOrderShoudSettlementAmount(OrderDTO orderMain, List<SettlementDetail> details) {
    OrderShoudSettlementAmount settlementAmount = new OrderShoudSettlementAmount();
    Map<Integer, Long> expenseTypeToFeeMoneyMap = Maps.newHashMap();
    for (SettlementDetail settlementDetail : details) {
        long feeMoney = Optional.ofNullable(expenseTypeToFeeMoneyMap.get(settlementDetail.getExpenseType())).orElse(0L);
        feeMoney += Optional.ofNullable(settlementDetail.getOassMoney()).orElse(0L);
        expenseTypeToFeeMoneyMap.put(settlementDetail.getExpenseType(), feeMoney);
    }
    long settlementMoney = SettlementMoneyCalcFeeInfoEnum.calcSettlementMoney(expenseTypeToFeeMoneyMap);
    long goodCommissionMoney = GoodCommissionMoneyCalcFeeInfoEnum.calcGoodCommissionMoney(expenseTypeToFeeMoneyMap);
    settlementAmount.setSettlementAmount(settlementMoney);
    settlementAmount.setGoodsCommission(goodCommissionMoney);
    settlementAmount.setOrderId(orderMain.getOrderId());
    settlementAmount.setOrgCode(orderMain.getOrgCode());
    settlementAmount.setStationNo(String.valueOf(orderMain.getDeliveryStationNo()));
    settlementAmount.setBillTime(new Date());
    settlementAmount.setRetSuccess(false);
    return settlementAmount;
}

The supporting enums encapsulate the calculation rules, eliminating duplicated if statements and centralizing the logic for easy updates:

enum SettlementMoneyCalcFeeInfoEnum {
    /** calculation items */
    FEE_33021(FeeInfoEnum.FEE_INFO_FREIGHT_ZS_NOSETTLE, "+"),
    FEE_33002(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE, "-"),
    FEE_32003(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ, "-"),
    FEE_32001(FeeInfoEnum.FEE_INFO_GOODS_NOSETTLE, "+"),
    FEE_31001(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE, "-"),
    FEE_34012(FeeInfoEnum.FEE_INFO_CHF_XSG_NOSETTLE, "+"),
    FEE_34013(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG, "-");
    private final FeeInfoEnum feeInfoEnum;
    private final String symbol;
    SettlementMoneyCalcFeeInfoEnum(FeeInfoEnum feeInfoEnum, String symbol) {
        this.feeInfoEnum = feeInfoEnum;
        this.symbol = symbol;
    }
    public static long calcSettlementMoney(Map<Integer, Long> expenseTypeToFeeMoneyMap) {
        long settlementMoney = 0L;
        for (SettlementMoneyCalcFeeInfoEnum calcFeeInfoEnum : SettlementMoneyCalcFeeInfoEnum.values()) {
            if ("+".equals(calcFeeInfoEnum.symbol)) {
                settlementMoney += Optional.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal())).orElse(0L);
            }
            if ("-".equals(calcFeeInfoEnum.symbol)) {
                settlementMoney -= Optional.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal())).orElse(0L);
            }
        }
        return settlementMoney;
    }
}

enum GoodCommissionMoneyCalcFeeInfoEnum {
    /** calculation items */
    FEE_33005(FeeInfoEnum.FEE_INFO_YFYJ_ZS_XSG),
    FEE_33002(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE),
    FEE_32003(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ),
    FEE_31001(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE),
    FEE_34013(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG);
    private final FeeInfoEnum feeInfoEnum;
    GoodCommissionMoneyCalcFeeInfoEnum(FeeInfoEnum feeInfoEnum) {
        this.feeInfoEnum = feeInfoEnum;
    }
    public static long calcGoodCommissionMoney(Map<Integer, Long> expenseTypeToFeeMoneyMap) {
        long goodCommissionMoney = 0L;
        for (GoodCommissionMoneyCalcFeeInfoEnum calcFeeInfoEnum : GoodCommissionMoneyCalcFeeInfoEnum.values()) {
            goodCommissionMoney += Optional.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal())).orElse(0L);
        }
        return goodCommissionMoney;
    }
}

The refactoring removes duplicated conditional logic, centralizes calculation rules, and separates data aggregation from business computation, thereby reducing divergent change and improving maintainability. The article concludes by reminding readers of the "broken‑window" effect in software engineering: neglecting small quality issues can lead to larger, harder‑to‑fix problems, so regular refactoring is essential for long‑term code health.