리팩토링 고려사항 정리

ㅎㅎ·2023년 9월 6일

리팩토링

1.setter를 지양

의도 불명확: setter라는 메소드 이름만으로 그 값을 왜 변경하는지 명확히 알 수가 없다. 내가 짠 코드를 나중에 다시 보거나, 다른 개발자가 본다고 했을 때 어떤 하나의 서비스 로직 메소드 내에 setter가 사용되고 있다고 하자. 해당 로직을 짤 당시에는 생각없이 값을 변경하기 위해 setter를 사용할 수 있겠지만, 나중에 다시 볼 때 코드를 이해하고자 한다면, 도대체 왜 여기서 이 값을 변경했는지 파악하기가 어렵다. 쉬운 로직이라면 가능하겠지만, 그게 아니라면, 혹은 남이 이걸 읽는다면 더더욱 어려울 것이다. 따라서 명확하게 값 변경의 의도를 알 수가 없기 때문에 지양해야 한다. 대신 정적 팩토리 메소드를 이용해서 값 변경의 의도를 메소드 이름에 명시해준 후에 사용하는 것이 좋다.
일관성 유지 불가: 이 또한 거대 개발 시스템, 다른 개발자들을 고려해서 생각해봐야 한다. setter를 어디에서나 무분별하게 사용할 수 있다면, 실수로 혹은 어떤 로직에서 필요해서 등 setter를 사용해서 객체의 값을 변경할 수 있다. 그러나 해당 객체의 해당 필드의 값이 항상 유지되어야 하는 등 일관성 유지가 필요하다면 이러한 일들로 인해서 일관성이 깨지게 된다. 따라서 setter를 통해 값을 쉽게 변경하지 못 하도록 하는 것이 좋다.
책임의 분산: 객체 지향 개발에서는 하나의 객체가 자신에게 관할된 모든 기능을 관리해야 한다. 객체 간 서로의 간섭 없이, 원하는 것을 서로에게 요청하는 형태로 프로그래밍이 이루어지기 때문이다. 그러나 setter를 사용하게 되면, 객체가 자신의 필드를 바꿀 수 있는 가능성을 다른 곳에 열어주게 된다. 이렇게 되면 다른 곳에서 해당 객체가 해야할 필드의 변경 작업을 대신 하게 되는데, 만약 객체의 필드가 바뀌는 등 내부 구조가 변경되면 setter를 호출했던 곳의 로직 또한 전부 변경이 필요하게 된다. 객체 간 간섭 없이 원하는 정보만 요청하여 편리하게 프로그래밍을 하려고 객체지향을 사용하는데, 서로 맞물려서 꼬여 있다면 객체지향의 장점이 사라지고 만다.

해결방법: setter를 없애고, 명확한 의도를 가진 메소드를 이용한다.

2.getter를 지양

getter는 보통 값을 조회하는 것에 그치지 않고, 조회한 값을 어떤 로직에 활용하기 마련이다. 만약 이렇게 getter를 통해 값을 조회하고, 이 값이 조건 검사를 위해 사용된다면 변경에 취약하다. 만약 해당 객체 내부에서 필드가 변경되어 기존에 getter를 이용해 가져오던 값이 사라지게 되면 조건 검사 로직 또한 수정되어야 한다.
해결방법: getter를 이용해 조건을 검사하지 말고, 이 조건을 검사한 결과를 리턴하는 메소드를 해당 객체에 생성하고, 단순히 이 메소드를 호출해서 사용하도록 변경.

참조:

1.생성자 주입을 사용하자.

생성자 주입

2.불변 객체 사용

다른 사람들이 코드를 읽고 수정할 때 final 객체는 안전성이 보장된다. final 변수는 한 번 초기화 되면 변경할 수 없고, final 객체는 한 번 초기화 하면 다른 객체로 다시 초기화 할 수 없다.(내부 필드 값은 final 변수가 아닌 이상 변경 가능) 따라서 해당 객체들을 다른 business logic에서 사용하고자 할 때 이 객체가 어디서 혹시 변경되어서 들어오면 어떻게 하지? 라는 걱정을 하지 않고 마음껏 사용 가능하다.

3.유틸성 또는 상수형 클래스는 내투 생성자를 통해 객체 생성 제한

유틸성 클래스는 전역에서 공유하는 기능성 클래스들을 말함. Token을 만드는 클래스라든지, DB를 연결하는 클래스라든지. 이러한 클래스들은 따로 객체를 생성해서 사용할 필요가 없고, 어디에서 사용하든 모두 같은 값들을 공유하면서 사용하는 것이 특징임. 따라서 내부 필드들이 모두 static으로 선언되어 있음. static을 이용한다는 것은 따로 객체를 생성할 필요가 없다는 것이기 때문에 외부에서 생성자를 사용할 일도 없음. 따라서 불필요하게 생성자를 사용할 수 있도록 코드를 열어두지 않기 위해 생성자를 직접 private으로 지정하거나 @NoArgsConstructor(access = AccessLevel.PRIVATE)를 이용해 제한해줘야 함.
KEY 값 같이 모든 곳에서 공유해서 쓰는 객체나 변수들은 static으로, 또한 변경이 아예 불가능한 것들은 final까지 붙여서 만들 것!

4.Controller를 최대한 가볍게

Spring에서 Controller는 클라이언트의 요청이 들어오고 나가는 곳이다. 요청을 처리하기 위한 비지니스 로직은 상당히 복잡할 수 있다. 하지만 이를 주고 받는 컨트롤러는 어떠한 데이터를 주고 받는지만을 명확하게 작성하는 것이 좋다. 그래야 다른 사람이 해당 API를 봤을 때 이해하기 쉬울 것이다.
참조: https://mangkyu.tistory.com/137

우리 프로젝트에서 보인 것들

1.try / catch 사용?

어떤 오류를 던지는지 명확하지가 않다. 그냥 try 블락 내에 모든 코드 중 runtime error가 발생하면 catch를 한다. 이 로직에서 어떤 에러가 발생할지 예측할 수가 없다. getReciptInfo 메소드.
try / chach 말고 thrwos를 통해 ExceptionHandler로 에러를 던지는 방식에 대한 고려..

2.변수명 변경

contentType.substring()이라는 메소드를 사용해 return한 결과를type 변수에 할당했는데 어떤 type을 가리키는지 알 수가 없다. 변수명을 명확히 수정할 필요가 있다.
중복되는 데 변경해야하는 변수명이 있다. 뭐 int roomCode로 받아서 String으로 변환해야 하는 경우, 이럴 때 어떤 변수명을 사용해야 좋을까?

3.메소드 활용

OCR API 이용 로직이 너무 길다. API에 맞는 값들을 지정해주는 거 같은데,, 뭘 지정해주는 건지... 흠... 이런 경우 어떻게 해야할지 고민이 필요해 보인다. 외부 API 문서에 대한 정보를 다시 직접 찾아볼 수 있도록 링크 참조를 남겨 두고, 값을 세팅하는 것에 대해서도 명확하게 좀 알 수 있도록 해야 할 거 같다.

어떤 변수명, 어떤 메소드명이 좋은지 찾아봐야 할 듯, 어떻게 바꿀지?

4.잘못된 메소드 명 사용

checkAndSaveMeetingMember 메소드에서 가입정원 초과 시 리턴이 BaseRESPONSE.customeSuccess로 이루어지고 있음

5.잘못된 return값?

setStartPlace 메소드에서 성공에 대한 return으로 body에 null을 리턴함. 메소드 오버로딩을 통해 body가 없는 메소드를 만들고 아무것도 리턴하지 않는게 낫지 않을까?
MeetingService에서 정원 초과시 -1 return하는 checkAndSaveMeetingMember -> 예외 상황에 대한 return 처리를 어떻게 할 것인지? 에러? 혹은 합의한 값? 뭐가 좋을까

6.긴 로직 모듈화?