[리팩토링] 가독성을 위한 리팩토링

피드백 내용

 

MemberDTO login = session.getAtrribute("login"); //현재 사용자 정보

이전까지 사용자 정보를 가져올때 이 문장을 계속 반복해서 썼었다.

나는 내가 쓴 코드라서 읽는데 불편함을 못느끼고 있었는데, 피드백을 받고보니 남이 봤을 땐 가독성이 떨어질 수 있겠다는 생각을 했다. 그리고 사용자 정보를 가져오는 반복되는 코드를 따로 private메서드로 만들어서 이름을 재정의하면 가독성을 높일 수 있다는 것은 생각도 못했었는데, 정말 좋은 방법인거 같다.

private MemberDTO getUserInfoBySession() {
	MemberDTO login = session.getAtrribute("login"); //현재 사용자 정보	
}

@RequestMapping(value = "/sendMail", method = RequestMethod.POST)
	public String sendMail(HttpSession session, HttpServletRequest request, @RequestParam("mail_file") MultipartFile multipartFile) {
		MemberDTO loginDto = getUserInfoBySession(session);
        .
        .
        .
}

그래서 바로 적용시켰다.

이렇게 하게되면 다른 곳에서 현재 사용자 정보를 가져올 때 getUserInfoBySession 메서드를 사용해서 가독성을 높일 수 있다.

saveFile이라는 메서드는 첨부파일을 저장하는 메서드이다.
MailService 내에서 공통으로 사용하기 위해 따로 빼놓은 메서드들을 위에다가 모아 놓았었는데, 오히려 가독성이 떨어진 것 같다. 그래서 피드백에 맞춰서 saveFile메서드를 처음 호출하는 메서드 바로 아래에 위치시켰다.

 

---

if(multipartFile.isEmpty) {
	.
    	.
    	.
}

위와 같은 조건문이 있었다.
첨부파일이 없는 경우에도 자꾸 DB에 경로가 저장되길래 DB의 공간을 차지하는 것 보다 null로 저장하는게 좋을 것 같아서 isEmpty를 통해 ture일 경우 경로를 null로 정했었다.

 

---

Early Return에 대해서 따로 정리를 했다.

https://hsch19.tistory.com/10

Early Return

 

---

위의 마지막 피드백에 맞게 Controller에서 addressListStr을 List<String>으로 파싱한 다음 Service로 넘겨주려 했으나,
내게 메일 쓰기의 경우, 프론트에서 mail_receiver(받는사람)에 해당하는 input태그가 없다.
즉, Controller에서 파싱할 때

String addressListStr = Request.getParameter(" mail_receiver ");

는 Null값을 가지게 된다.

고로, List<String>으로 만들어주기 위한 

List<String> addresList = Arrays.asList(addressListStr.split(" ")));

위의 코드에서 NullPointException이 터지게 된다.

그래서 isEmpty를 사용해서 조건문을 만드려고 했으나, null 비교로 냅두게 됐다.

 

그리고 람다식을 사용해서 처리를 해보려고 했으나,

아래 for문 코드는 List 두개를 동시에 순회하고 하나의 DTO에 값을 넣어주고 있기 때문에 람다식의 forEach는 사용하지 못할 것 같아서 현상을 유지하기로 했다. 

//메일 보내기
public String sendMail(MailDTO mailDto, String addressListStr, MultipartFile attachmentFile) {
    System.out.println("attachmentFile : " + attachmentFile.isEmpty());
    if(!attachmentFile.isEmpty()) {
        saveFile(mailDto, attachmentFile);
    }
    dao.saveMail(mailDto);

    //보낸 메일 고유번호
    int recentMailNum = mailDto.getMail_num();
    //DB에 넘겨줄 MailRecDto
    MailRecDTO mailRecDto = new MailRecDTO();
    mailRecDto.setMail_num(recentMailNum);

    //Mail_Rec테이블 insert (MappingTable -- 받은 사람들 저장하기)
    String msg = "메일전송이 완료되었습니다!";
    int res = 0;
    int resCheck = 0;

    //내게쓰기인 경우
    if(addressListStr == null) {
        mailRecDto.setRec_num(mailDto.getMember_num());
        mailRecDto.setMail_receiver(mailDto.getMail_sender()); //보내는 사람이 받는 사람이기 떄문
        dao.saveReceiveTable(mailRecDto); //insert 작업
        return msg;
    }

    if(addressListStr != null) {
        List<String>addressList = Arrays.asList(addressListStr.split(" "));
        List<Integer> rec_numList = dao.findMemberNumByMailAddress(addressList);
        resCheck = rec_numList.size();

        for(int i=0; i<rec_numList.size(); i++) {
            mailRecDto.setRec_num(rec_numList.get(i));
            mailRecDto.setMail_receiver(addressList.get(i));
            res += dao.saveReceiveTable(mailRecDto); //insert 작업
        }
        return msg;
    }

    if(res != resCheck) {
        msg = "다시 시도해주세요";
        return "redirect:writeMail";
    }

    return msg;
}

 

대신 DAO에서 List를 가지고 처리하는 메서드에 람다식으로 표현을 써봤다

//메일주소 리스트로 멤버번호 리스트 찾기
public List<Integer> findMemberNumByMailAddress(List<String> addressList) {
    List<Integer> rec_numList = new ArrayList<>();

    addressList.forEach(address -> rec_numList.add(session.selectOne("findMemberNumByMailAddress",address)) );
    return rec_numList;
}

forEach로 List안에 저장된 String을 순회한다.

 

정리

1. 가독성이 떨어지는 코드는 따로 private 메서드를 만들어서 이름을 정의해줄 수 있다.

2. 따로 메서드를 빼놓은 경우, 해당 메서드 아래에 정의를 해준다.

3. 중첩 if문 혹은 if-else문의 경우, Eerly Return 을 사용해서 가독성을 높일 수 있다.

4. 람다식을 사용해서 코드의 양을 줄일 수 있다.