[쿠링] 중복코드를 Template Method Pattern으로 Refactoring 하기

해당 글은 개인 프로젝트를 개선해 나가면서 내용을 정리하는 글입니다.

 

1. 현 상황 (개선하기 전의 코드)

우선 다음 코드는 Notice를 Scarp 하는 코드입니다.

문제는 (scarp, scarpAll), (requestWithDeptInfo, requestAllPageWithDeptInfo) 간의 중복 코드가 너무나 많다는 점입니다.

scarp : 최근 공지 조회

scarpAll : 모든 공지 조회

이렇게 2개의 메서드를 구분하다 보니 발생한 중복 코드였습니다.

 

템플릿 메서드 패턴, 함수형 인터페이스, 람다식을 통하여 중복을 제거할 생각입니다!

 

우선 개선하기 전의 코드는 다음과 같습니다!

@Slf4j
@Component
@NoArgsConstructor
public class DepartmentNoticeScraper {

    public List<CommonNoticeFormatDto> scrap(DeptInfo deptInfo) throws InternalLogicException {
        List<ScrapingResultDto> requestResults = requestWithDeptInfo(deptInfo);

        log.info("[{}] HTML 파싱 시작", deptInfo.getDeptName());
        List<CommonNoticeFormatDto> noticeDtoList = htmlParsingFromScrapingResult(deptInfo, requestResults);
        log.info("[{}] HTML 파싱 완료", deptInfo.getDeptName());

        log.info("[{}] 공지 개수 = {}", deptInfo.getDeptName(), noticeDtoList.size());
        if (noticeDtoList.size() == 0) {
            throw new InternalLogicException(ErrorCode.NOTICE_SCRAPER_CANNOT_SCRAP);
        }

        return noticeDtoList;
    }

    public List<CommonNoticeFormatDto> scrapAll(DeptInfo deptInfo) throws InternalLogicException {
        List<ScrapingResultDto> requestResults = requestAllPageWithDeptInfo(deptInfo);

        log.info("[{}] HTML 파싱 시작", deptInfo.getDeptName());
        List<CommonNoticeFormatDto> noticeDtoList = htmlParsingFromScrapingResult(deptInfo, requestResults);
        log.info("[{}] HTML 파싱 완료", deptInfo.getDeptName());

        log.info("[{}] 공지 개수 = {}", deptInfo.getDeptName(), noticeDtoList.size());
        if (noticeDtoList.size() == 0) {
            throw new InternalLogicException(ErrorCode.NOTICE_SCRAPER_CANNOT_SCRAP);
        }

        return noticeDtoList;
    }

    private List<ScrapingResultDto> requestWithDeptInfo(DeptInfo deptInfo) {
        long startTime = System.currentTimeMillis();

        log.info("[{}] HTML 요청", deptInfo.getDeptName());
        List<ScrapingResultDto> reqResults = deptInfo.scrapLatestPageHtml();
        log.info("[{}] HTML 수신", deptInfo.getDeptName());

        long endTime = System.currentTimeMillis();
        log.info("[{}] 파싱에 소요된 초 = {}", deptInfo.getDeptName(), (endTime - startTime) / 1000.0);

        return reqResults;
    }

    private List<ScrapingResultDto> requestAllPageWithDeptInfo(DeptInfo deptInfo) {
        long startTime = System.currentTimeMillis();

        log.info("[{}] HTML 요청", deptInfo.getDeptName());
        List<ScrapingResultDto> reqResults = deptInfo.scrapAllPageHtml();
        log.info("[{}] HTML 수신", deptInfo.getDeptName());

        long endTime = System.currentTimeMillis();
        log.info("[{}] 파싱에 소요된 초 = {}", deptInfo.getDeptName(), (endTime - startTime) / 1000.0);

        return reqResults;
    }
    
    private List<CommonNoticeFormatDto> htmlParsingFromScrapingResult(DeptInfo deptInfo, List<ScrapingResultDto> requestResults) {
        // 생략
    }
}

 

2. 개선 후의 코드

중복이 되지 않는 부분과 중복되는 부분은 분리하여 봅시다!

 

2 - 1) 중복되는 부분

개발자가 전달할 부분과(중복되지 않는 부분), 일종의 template 역할을 할 중복코드 부분을 메서드를 통해 분리해 보자!

우선, 중복되는 Template 코드부터 살펴보자!

 

▶ DepartmentNoticeScraperTemplate (중복되는 부분)

@Slf4j
@Component
public class DepartmentNoticeScraperTemplate {

    public List<CommonNoticeFormatDto> scrap(DeptInfo deptInfo, Function<DeptInfo, List<ScrapingResultDto>> decisionMaker) throws InternalLogicException {
        List<ScrapingResultDto> requestResults = requestWithDeptInfo(deptInfo, decisionMaker);

        log.info("[{}] HTML 파싱 시작", deptInfo.getDeptName());
        List<CommonNoticeFormatDto> noticeDtoList = htmlParsingFromScrapingResult(deptInfo, requestResults);
        log.info("[{}] HTML 파싱 완료", deptInfo.getDeptName());

        log.info("[{}] 공지 개수 = {}", deptInfo.getDeptName(), noticeDtoList.size());
        if (noticeDtoList.size() == 0) {
            throw new InternalLogicException(ErrorCode.NOTICE_SCRAPER_CANNOT_SCRAP);
        }

        return noticeDtoList;
    }

    private List<ScrapingResultDto> requestWithDeptInfo(DeptInfo deptInfo, Function<DeptInfo, List<ScrapingResultDto>> decisionMaker) {
        long startTime = System.currentTimeMillis();

        log.info("[{}] HTML 요청", deptInfo.getDeptName());
        List<ScrapingResultDto> reqResults = decisionMaker.apply(deptInfo);
        log.info("[{}] HTML 수신", deptInfo.getDeptName());

        long endTime = System.currentTimeMillis();
        log.info("[{}] 파싱에 소요된 초 = {}", deptInfo.getDeptName(), (endTime - startTime) / 1000.0);

        return reqResults;
    }

    private List<CommonNoticeFormatDto> htmlParsingFromScrapingResult(DeptInfo deptInfo, List<ScrapingResultDto> requestResults) {
        List<CommonNoticeFormatDto> noticeDtoList = new LinkedList<>();
        for (ScrapingResultDto reqResult : requestResults) {
            Document document = reqResult.getDocument();
            String viewUrl = reqResult.getUrl();

            List<String[]> parseResult = deptInfo.parse(document);
            for (String[] oneNoticeInfo : parseResult) {
                noticeDtoList.add(CommonNoticeFormatDto.builder()
                        .articleId(oneNoticeInfo[0])
                        .postedDate(oneNoticeInfo[1])
                        .subject(oneNoticeInfo[2])
                        .fullUrl(viewUrl + oneNoticeInfo[0])
                        .build());
            }
        }
        return noticeDtoList;
    }
}

기존의 코드에서 딱 중복되어 사용되던 부분만 template로 이동시켰다!

 

잘 보면, scrap의 인자로 Function<T, P> 를 받고 있다.

원래였다면 메서드를 하나 정의하고 있는 인터페이스를 하나 만들어서 abstract DepartmentNoticeScraperTemplate로 사용했겠지만, 나 같은 경우 Lamda 식을 활용하여 템플리 메서드 페턴을 사용하고 있다.

 

꼭 추상클래스로 중복을 제거하지 않아도, 함수형 인터페이스인 Function, Consumer, Supplier 같은 것 을 사용하면 중복 제거에 매우 효과적이라 생각한다.

 

2 - 2) 중복되지 않는 부분

다음과 같이 함수를 호출할 때 원하는 메서드를 선택하여 인자로 전달할 수 있게 되었습니다!!

updateDepartmentAsync(deptInfo, DeptInfo::scrapLatestPageHtml) // 최신 공지
updateDepartmentAsync(deptInfo, DeptInfo::scrapAllPageHtml) // 모든 공지

private List<CommonNoticeFormatDto> updateDepartmentAsync(DeptInfo deptInfo, Function<DeptInfo, List<ScrapingResultDto>> decisionMaker) {
    List<CommonNoticeFormatDto> scrapResults = scrapperTemplate.scrap(deptInfo, decisionMaker);
    Collections.reverse(scrapResults);
    return scrapResults;
}