FullCalender 리팩토링 & 테스트케이스 -1

드디어 4학년 1학기가 종강을 하고.. 졸업 작품도 점점 막바지를 향해 다가가고 있다.

이번 여름 방학 동안

주요기능 한 두개 정도 구현을 하면 거의 마무리가 되지 않을까 하는 생각이다.

 

사실 지금 짠 코드들은 좀 문제가 많다.. 오류가 나더라도 어디서 왜 오류가 났는지

알 수 없는 오류도 간혹 발생하고, Optional 에 대한 개념도 없이 비싼 리소스를 단지 .get() 을 통해서 불러오는

안좋은 사용법이라던가.. JPA 에서도 그저 기본 기능을 이용하려고 findBy~~~~ 가 엄청 길어지는 문제라거나..

 

결정적으로 제대로 된 테스트 케이스도 작성하지 않았다.

처음에는 TDD (테스트주도개발) 방법론을 사용해서 테스트 코드를 많이 작성해야지!.. 하는 생각이였지만

기능 개발도 진행하며 학교 수업도 병행하고, 스프링&JPA 에 대한 개념을 공부도 해야하고..

결국 핑계지만 이런 저런 이유로 시간을 많이 뺏기다 보니 테스트 코드를 거의 작성하지 않은 채로 마무리 단계로 달려가고 있었다.

 

팀원들 모두 이러한 문제점을 인식하고 약 2주 가량은 리팩토링과 테스트 코드를 작성하는데에 전념하고자 의견을 나눴다.

사실 리팩토링은 정답을 알고 있어야 아 이게 문제지 하고 고칠 수가 있다고 생각하는데

내 수준에서는 아직 그러한 문제점이 많이 발견되지가 않는다.

 

그렇지만 우선 할 수 있는 것들을 해보고자 한다!..

 

리팩토링

---

JPA 리팩토링

 

기존의 코드에서 FullCalendar 를 이용해서 스케줄을 수정하거나 수정 요청을 보내는 경우 , 

수정되기 이전의 스케줄을 조회하고, 바꾸고자 하는 스케줄을 그 위에 덮어 씌우는 방식으로 작동하고 있었다.

그러기 위해서는 수정되기 이전의 스케줄을 필수로 조회해야 하는데, 조회를 위한 코드를 다음과 같이 사용하고 있었다.

코드 상의 문제도 없고, 직관적으로 무엇을 위한 코드인지도 알 수 있지만 실제로 사용하기에는 너무 긴 코드이다.

'findByUserAndScheduleDateTimeStartAndScheduleDateTimeEnd'

그렇기에 @Query 문을 이용해서 내가 직접 코드를 작성하고 코드 명을 간략하게 'getBeforeSchedule' 로 바꿨다.

@Query 를 오랜만에?.. 작성하기도 했고 별로 사용한 기억이 없는지라 조금 헤맸다가 구글링을 통해 잘 작성하였다.

@Param 을 통해 내가 입력받고자 하는 데이터를 미리 파라미터 처리해야한다는 것을 기억해두자!..

 

 

 

Optional 리팩토링

 

가장 문제가 많은 녀석이다. Optional 을 사용하면 Null 체크가 편해진다는 이야기를 보고 '와 안쓰면 바보!' 하고

Optional 을 마구 남발하며 수 많은 코드에서 .get() 으로 작성하고 있었다.

예를 들면 이런식이다.

 

사실 자바 8 문법을 공부하지 않아서 Stream 이나 람다, Opitonal 등등.. 사용법을 모르고 있다.

날 잡고 공부해야지 해야지 하면서 기존의 코드로 작성은 할 수 있으니까.. 하고 그냥 작성을 하고 있었는데 이번 기회를 빌어

Optional 에 대한 사용법이라도 제대로 알고 가고자 한다.

 

Optional 사용법

'Clean Code' 에서 위와 같은 예제가 존재한다고 한다!..

조건마다 != null 을 이용해서 검증을 하는데 단순한 코드라면 작성이 편하지만,

조건이 많아질수록 계속 if~ 를 통해 null 을 체크하기는 힘들 것이다.

또한 null 을 반환하는 것도 그렇고, null 을 인수로 전달하는 것도 피해야 할 안티패턴이다.

위의 방법은 결국 NullPointerException 을 일으키게 할 여지가 충분하다!

이를 위해서 Optional 을 사용한다.

 

java.util.Optional<T> 클래스

 

Optional 클래스는 'T'타입의 객체를 포장해 주는 래퍼 클래스(Wrapper class)

• 따라서 Optional 인스턴스는 모든 타입의 참조 변수를 저장할 수 있음
• NullPointerException 예외를 제공되는 메소드로 간단히 회피
• 만약 Optional 객체에 저장된 값이 null이면, NoSuchElementException 예외가 발생

 

1. isPresent()-get() 대신 orElse()/orElseGet()/orElseThrow()

2. orElse(new ...) 대신 orElseGet(() -> new ...)

orElse 는 반드시 실행되는 반면 , orElseGet() 은 null 인 경우에만 실행된다 !

 

 

3. 단지 값을 얻을 목적이라면 Optional 대신 null 비교

 

4. Optional을 필드로 사용 금지

추가적인 사항으로는 생성자, 수정자, 파라미터 등으로 Optional 을 넘기지 않는 것이다.

파라미터는 반환 타입으로 대체 동작을 사용하기 위해 고안된 것이므로 오직 반환 값을 위해서

사용하는 것을 권장한다.

8. of(), ofNullable() 혼동 주의

그 외에도 주의 사항이 여러가지 있지만 우선 내 수준에서 중요한 내용은 이정도 인 것 같다.

Optional 은 비싼 리소스이기 때문에 사용을 지양하는 방식을 취하며 필요한 경우에만 작성하자!

---

수정 코드

 

 

수정 전

수정 전의 코드는 보다시피 .get() 을 통해 값을 가져오고 있어서 orElseThrow 를 사용해서 작성해보고자 했다.

수정 후

기존의 코드는 컨트롤러 부분에서 예외처리를 하고 있었는데

바꾸면서 좀 더 나은 코드가 된 느낌?!..

---

수정 전

Optional 을 사용한 대표적인 안좋은 예제가 순식간에 다 나와버린 코드이다.!..

1차 수정

1번째로 userService.get() 으로 바로 반환하던것을 orElseThrow 로 예외처리를 해주었다!

첫번째는 무난하게 처리했으나 두번째를 수정하는 부분에서 문제가 있었다.

1번 스케줄에 수정 요청이 없는 경우, ScheduleUpdateReq 테이블에 수정 요청을 보내주고,

이미 1번 스케줄에 대해 스케줄 요청이 보내진 경우, 해당 수정 요청을 Update 시켜주는 코드이다.

 

Optional 을 사용해서 Return 하려 했는데 orElseThrow() 를 통해 예외처리를 할 필요가 없었고

(null 값이 나올 수 있음을 예상했고, 해당 코드에서는 null 값이 발생한다고 해서 오류가 생긴것이 아니기 때문이다.),

orElseGet() 을 통해 새로운 객체를 생성하거나 대체 값을 넣는 코드도 아니기에 넣을 수 있는 선택지가 null 밖에 없다고 생각했다.

.orElseGet을 통해 null 을 반환할거면 굳이 비싼 Optional 을 사용할 필요 없이 ID 를 반환해서 null 체크를 하자고 생각했고,

그 결과 아래와 같이 findByAssignSchedule 은 Integer 타입으로 반환하도록 수정하고 null 인 경우 Save,

null 이 아닌 경우 Update 로 변경하였다.

 

2차 수정 후

 

이 외에는 위 코드들과 동일한 부분 수정의 반복이라 따로 글로 작성하지 않고 수정하였다.

---

사담

 

음.. 리팩토링이 코드로 기능을 구현하는 것보다 더 어려운 것 같다.

좋은 코드가 무엇인지 잘 알지 못하다보니 고민을 해서 답을 도출해도 이게 맞는 건지 틀린건지..

그래도 고민을 해보고 내가 결론을 내렸다는 것에 의의를 두고 작성을 해보았다.

마음에 걸렸던 코드들은 우선 개선을 해보았으니 다음에는 테스트 코드 작성을 해보고자 한다!..

먼 훗날에는 슉슉 멋지게 리팩토링 할 수 있겠지!..

 

---

참고 블로그

https://homoefficio.github.io/2019/10/03/Java-Optional-%EB%B0%94%EB%A5%B4%EA%B2%8C-%EC%93%B0%EA%B8%B0/

Java Optional 바르게 쓰기

https://mangkyu.tistory.com/70

[Java] Optional이란? Optional 개념 및 사용법 - (1/2)