[리팩토링] 레거시 코드와 SRP

처음에는 단순했다.

// 최초저장
if (vo.getFyerSplyBzentyAuditSn() == null || vo.getFyerSplyBzentyAuditSn().isEmpty()) {
    fyerSplyBzentyChckPlanWrtMDao.saveChkPlan(vo);
    processAddedSchedules(vo, true);
} else {
    //수정
    fyerSplyBzentyChckPlanWrtMDao.updateChkPlan(vo);
    processAddedSchedules(vo, false);
    processEditedSchedules(vo);
    processRemovedSchedules(vo);
}

• 키값이 없으면 최초저장, 있으면 수정 정도만 있었는데 공통승인 로직까지 생각하게 되니 점점 복잡해졌다.

그 결과

메서드 하나에 모든 로직이 들어갔다.

@Service
public class FyerSplyBzentyChckPlanWrtMServiceImpl implements FyerSplyBzentyChckPlanWrtMService {

    @Resource(name = "commonFunc")
    private CommonFunc commonFunc;

    private final FyerSplyBzentyChckPlanWrtMDao fyerSplyBzentyChckPlanWrtMDao;
    private final CmAprvService cmAprvServiceImpl;

    @Autowired
    private FyerSplyBzentyChckSchMDao fyerSplyBzentyChckSchMDao;

    public FyerSplyBzentyChckPlanWrtMServiceImpl(
            FyerSplyBzentyChckPlanWrtMDao fyerSplyBzentyChckPlanWrtMDao,
            CmAprvService cmAprvServiceImpl) {
        this.fyerSplyBzentyChckPlanWrtMDao = fyerSplyBzentyChckPlanWrtMDao;
        this.cmAprvServiceImpl = cmAprvServiceImpl;
    }

    // 저장
    @Override
    public FyerSplyBzentyChckMVo saveFyerSplyBzentyChckPlanM(FyerSplyBzentyChckMVo vo) {
        // 승인 처리 (승인 요청 시에만)
        if ("aprv".equals(vo.getSaveGbn())) {
            processApproval(vo);
        } else {
            // 일반 저장 시 진행상황 코드 설정
            if (commonFunc.isEmpty(vo.getSplymanPrgrsSittnCd())) {
                vo.setSplymanPrgrsSittnCd(ApprovalEnum.ANNUAL_PLAN_WRITING.getCode());
            }
        }

        // 최초저장
        if (vo.getFyerSplyBzentyAuditSn() == null || vo.getFyerSplyBzentyAuditSn().isEmpty()) {
            fyerSplyBzentyChckPlanWrtMDao.saveChkPlan(vo);
            processAddedSchedules(vo, true);
        } else {
            //수정
            fyerSplyBzentyChckPlanWrtMDao.updateChkPlan(vo);
            processAddedSchedules(vo, false);
            processEditedSchedules(vo);
            processRemovedSchedules(vo);
        }

        // 승인요청정보가 있을 때만 승인정보 생성 및 업데이트
        if (vo.getAprvInfoList() != null && !vo.getAprvInfoList().isEmpty()) {
            // 신규 승인정보 저장 → aprvSn 생성
            cmAprvServiceImpl.saveAprv(vo);
            // aprvSn을 세팅
            vo.setPlanAprvSn(vo.getAprvSn());
            // 승인 SN을 메인 테이블에 업데이트
            fyerSplyBzentyChckPlanWrtMDao.updatePlanAprvSn(vo);
        }

        return vo;
    }

    private void processApproval(FyerSplyBzentyChckMVo vo) {
        if (vo.getAprvInfoList() != null && !vo.getAprvInfoList().isEmpty()) {
            // 신규 승인정보 저장 → aprvSn 생성
            cmAprvServiceImpl.saveAprv(vo);

            // aprvSn을 세팅
            vo.setPlanAprvSn(vo.getAprvSn());

            // 진행상황을 "계획 승인 요청"으로 변경
            vo.setSplymanPrgrsSittnCd(ApprovalEnum.ANNUAL_PLAN_APPROVAL.getCode());

            // 승인 상신 (진행상황 코드 업데이트)
            cmAprvServiceImpl.aprvRequest(vo);

            // 승인 SN을 메인 테이블에 업데이트
            fyerSplyBzentyChckPlanWrtMDao.updatePlanAprvSn(vo);
        }
    }

    private void processAddedSchedules(FyerSplyBzentyChckMVo vo, boolean isNew) {
        if (vo.getAddedPlanList() != null && !vo.getAddedPlanList().isEmpty()) {
            for (FyerSplyBzentyChckMVo schedule : vo.getAddedPlanList()) {
                schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
                if (isNew) {
                    fyerSplyBzentyChckPlanWrtMDao.saveChkSchedule(schedule);
                } else {
                    fyerSplyBzentyChckPlanWrtMDao.insertChkSchedule(schedule);
                }
            }
        }
    }

    private void processEditedSchedules(FyerSplyBzentyChckMVo vo) {
        if (vo.getEditedPlanList() != null && !vo.getEditedPlanList().isEmpty()) {
            for (FyerSplyBzentyChckMVo schedule : vo.getEditedPlanList()) {
                schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
                fyerSplyBzentyChckPlanWrtMDao.updateChkSchedule(schedule);
            }
        }
    }

    private void processRemovedSchedules(FyerSplyBzentyChckMVo vo) {
        if (vo.getRemovedPlanList() != null && !vo.getRemovedPlanList().isEmpty()) {
            for (FyerSplyBzentyChckMVo schedule : vo.getRemovedPlanList()) {
                schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
                schedule.setDelYn("Y");
                fyerSplyBzentyChckPlanWrtMDao.updateChkSchedule(schedule);
            }
        }
    }

    // 계획 삭제
    @Override
    public FyerSplyBzentyChckMVo delFyerSplyBzentyChckM(FyerSplyBzentyChckMVo vo) {
        fyerSplyBzentyChckSchMDao.deleteChkPlan(vo);
        return vo;
    }

    // 계획 승인요청
    @Override
    public FyerSplyBzentyChckMVo aprvFyerSplyBzentyChckM(FyerSplyBzentyChckMVo vo) {
        // 진행상황 코드 업데이트
        vo.setSplymanPrgrsSittnCd(ApprovalEnum.ANNUAL_PLAN_APPROVAL.getCode());
        fyerSplyBzentyChckPlanWrtMDao.updateChkPlan(vo);
        return vo;
    }

}

이건 아닌거 같아서 도움을 요청했다.

OOP.. 냄새 정도는 맡기는 했으나, 실무레벨에서는 어떻게 적용할지 감이 안왔다.

회사선배는 전형적인 절차지향 코드라 했다.

문제점

SRP 위반

• 하나의 메서드(saveFyerSplyBzentyChckPlanM)가 너무 많은 책임을 가지고 있다 승인 분기 처리, 상태코드 초기화, 저장/수정 분기, DB저장 등… 많다! 각자 한 가지 책임만 가지게 한다.
• 읽기가 너무 힘들다

추상화 레벨 불일치

• 고수준, 저수준 로직이 뒤석여 있어 코드가 눈에 잘 안들어온다. 같은 추상화 레벨로 통일한다 위에서 아래로 점점 구체화 한다.

문자열 비교로 분기

• “aprv”, “saveGbn”, isNew 같은 플래그로 동작이 결정된다.

절차지향적 사고

• 할 일을 순서대로 나열 했을 뿐, 책임분리를 지키지 못했다.

그 외 의미를 알 수 없는 이름들..

리펙토링 후

// 저장
@Override
public FyerSplyBzentyChckMVo saveFyerSplyBzentyChckPlanM(FyerSplyBzentyChckMVo vo) {
    initProgressStatusCodeIfNotPresent(vo);                //진행상황 코드가 없으면 초기화
    addAnlSupPlanWrt(vo.getFyerSplyBzentyAuditSn(), vo);   //저장
    preRegisterApprovalRequestInfo(vo);                    //승인정보등록
    return vo;
}

• 각자 한 가지 책임만 가지게 한다.
• 같은 추상화 레벨로 통일 한다.
• 위에서 아래로 점점 구체화 한다.

전체 코드

이제 좀 봐줄만하다 (아직 개선할 점은 많지만..)

@Service
public class FyerSplyBzentyChckPlanWrtMServiceImpl implements FyerSplyBzentyChckPlanWrtMService {

    @Resource(name = "commonFunc")
    private CommonFunc commonFunc;

    private final FyerSplyBzentyChckPlanWrtMDao fyerSplyBzentyChckPlanWrtMDao;
    private final CmAprvService cmAprvServiceImpl;

    @Autowired
    private FyerSplyBzentyChckSchMDao fyerSplyBzentyChckSchMDao;

    public FyerSplyBzentyChckPlanWrtMServiceImpl(FyerSplyBzentyChckPlanWrtMDao fyerSplyBzentyChckPlanWrtMDao, CmAprvService cmAprvServiceImpl) {
        this.fyerSplyBzentyChckPlanWrtMDao = fyerSplyBzentyChckPlanWrtMDao;
        this.cmAprvServiceImpl = cmAprvServiceImpl;
    }

    // 저장
    @Override
    public FyerSplyBzentyChckMVo saveFyerSplyBzentyChckPlanM(FyerSplyBzentyChckMVo vo) {
        initProgressStatusCodeIfNotPresent(vo);
        addAnlSupPlanWrt(vo.getFyerSplyBzentyAuditSn(), vo);
        preRegisterApprovalRequestInfo(vo);
        return vo;
    }

    private void initProgressStatusCodeIfNotPresent(FyerSplyBzentyChckMVo vo) {
        if (StringUtils.isEmpty(vo.getSplymanPrgrsSittnCd())) {
            vo.setSplymanPrgrsSittnCd(ApprovalEnum.ANNUAL_PLAN_WRITING.getCode());
        }
    }

    private void addAnlSupPlanWrt(String FyerSplyBzentyAuditSn, FyerSplyBzentyChckMVo vo) {
        if (FyerSplyBzentyAuditSn == null || FyerSplyBzentyAuditSn.isEmpty()) {
            createNewAccount(vo);
            return;
        }
        updateAnlSupPlanWrt(vo);
    }

    private void createNewAccount(FyerSplyBzentyChckMVo vo) {
        fyerSplyBzentyChckPlanWrtMDao.addAnnualSupplierPlan(vo);
        addAnlSupResult(vo, true);
    }

    private void updateAnlSupPlanWrt(FyerSplyBzentyChckMVo vo) {
        fyerSplyBzentyChckPlanWrtMDao.updateAnnualSupplierPlan(vo);
        addAnlSupResult(vo, false);
        updateAnlSupResult(vo);
        deleteAnlSupResult(vo);
    }

    private void preRegisterApprovalRequestInfo(FyerSplyBzentyChckMVo vo) {
        cmAprvServiceImpl.saveAprv(vo);
        vo.setPlanAprvSn(vo.getAprvSn());
        fyerSplyBzentyChckPlanWrtMDao.updateApprovalSerial(vo);
    }

    private void addAnlSupResult(FyerSplyBzentyChckMVo vo, boolean isNew) {
        if (vo.getAddedPlanList() == null || vo.getAddedPlanList().isEmpty()) {
            return;
        }

        for (FyerSplyBzentyChckMVo schedule : vo.getAddedPlanList()) {
            schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
            if (isNew) {
                fyerSplyBzentyChckPlanWrtMDao.addCheckSchedule(schedule);
            }
        }
    }

    private void updateAnlSupResult(FyerSplyBzentyChckMVo vo) {
        if (vo.getEditedPlanList() == null || vo.getEditedPlanList().isEmpty()) {
            return;
        }

        for (FyerSplyBzentyChckMVo schedule : vo.getEditedPlanList()) {
            schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
            fyerSplyBzentyChckPlanWrtMDao.updateCheckSchedule(schedule);
        }
    }

    private void deleteAnlSupResult(FyerSplyBzentyChckMVo vo) {
        if (vo.getRemovedPlanList() == null || vo.getRemovedPlanList().isEmpty()) {
            return;
        }

        for (FyerSplyBzentyChckMVo schedule : vo.getRemovedPlanList()) {
            schedule.setFyerSplyBzentyAuditSn(vo.getFyerSplyBzentyAuditSn());
            schedule.setDelYn("Y");
            fyerSplyBzentyChckPlanWrtMDao.updateCheckSchedule(schedule);
        }
    }

    //승인요청
    @Override
    public FyerSplyBzentyChckMVo aprvFyerSplyBzentyChckM(FyerSplyBzentyChckMVo vo) {
        anlSupPlanWrtEscalate(vo);
        return vo;
    }

    private void anlSupPlanWrtEscalate(FyerSplyBzentyChckMVo vo) {
        addAnlSupPlanWrt(vo.getFyerSplyBzentyAuditSn(), vo);
        vo.setSplymanPrgrsSittnCd(ApprovalEnum.ANNUAL_PLAN_APPROVAL.getCode());
        fyerSplyBzentyChckPlanWrtMDao.updatePlanProgressStatus(vo);

        cmAprvServiceImpl.aprvRequest(vo);
    }

    @Override
    public FyerSplyBzentyChckMVo delFyerSplyBzentyChckM(FyerSplyBzentyChckMVo vo) {
        fyerSplyBzentyChckSchMDao.deleteChkPlan(vo);
        return vo;
    }

}

좀 더 개선할 점

• 단일 vo 사용
  • 현재 단일 VO를 여러 메서드에서 돌려 쓰고있다.
  • 의존성 불명확 → 메서드가 VO의 어떤 필드를 사용하는지 모른다.
  • 예상 못한 참조 → 다른 메서드가 수정한 값을 모르고 참조할 수 있다 → 디버깅 지옥
  • 테스트 어려움
  • 승인 로직이 복잡해 단일 VO를 사용했지만 DTO로 분리해 사용하는 습관을 들이자.
• 메서드 명 개선
  • 메서드명만 개선해도 주석을 달 필요가없다!

배우고 느낀점

• 설계가 매우매우매우x100 중요하다.
• 코드가 길어진다고 어려워지는건 아니다. 짧은 코드를 만드려 애쓰지 말자.
• 내가 만드는 로직 만큼은 생각좀 하고 만들자
• 적당한 중복은 괜찮다.
• 극한까지 구체화하면 메서드 하나에 동작 하나 → ? 뭐든 적당히..