중간 발표 이후 리팩토링 전 코드 리뷰 답변 #777
sangwonsheep
started this conversation in
General 일반 공유
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
질문 링크
https://www.notion.so/Final_-_2-Tech-Pick-a9ff046b8bb741a59f0ee217e21b8596
답변
Q1. 인프라 스트럭처 구현체 코드에서의 검증 로직
어떤 검증이냐에 따라서 어디에 위치할것인가가 다른게 맞을것 같아요.
예를들어서 어떤 값이 비즈니스적으로 올바른 값인지 검증 하는것(ex> 나이는 음수가 될 수 없다)은 당연히 서비스레이어에서 검증이 이뤄져야 할것이구요
그게 아니라 인프라스트럭처의 관심사의 영역(ex> upsert - update + insert, 이 기능을 위해서 서비스레이어에서 값이 있는지 없는지 체크해서 update or insert를 호출 하는게 아니라, 서비스에서는 단순히 upsert를 호출하고, 인프라스트럭처에서 실제 구현을 담당)이라면 인프라스트럭처에서 해야 하구요.
이걸 구분하는 좋은 방법중 하나는, 사용하는 인프라를 바꿀때 영향이 있을것인가? 를 생각해보는것이에요. 사용하는 인프라를 바꿀때 같이 바뀌어야 하는 부분은 인프라스트럭처에서, 아니라면 비즈니스영역에 구현 해두면 됩니다
Q2. User정보를 파라미터로 받을것인가
자 이 메서드를 테스트를 한다고 가정 해봅시다.
시큐리티로부터 user를 전달 받는 경우, 시큐리티를 Mock으로 만들던지 해야겟죠? 이렇게 되는 코드는 테스트 가용성이 떨어지는 코드라고 말할수 있어요.
반면에 파라미터로 전달받게 된다면 파라미터에서 가짜 user만 만들어서 넣어주면 되는것이죠.
따라서 이 부분은 파라미터로 빼는게 테스트적인 측면에서 더 좋다고 말할수 있어요.
추가적으로, 지금 정의된 기능은 어쨋든 “로그인한 사용자” 에 대해서만 동작하기에 서비스 메서드로 빠질 이유가 없다고 보았습니다. 라고 말씀 해주셨는데, 이 로그인한 사용자라는 관심사는 Controller의 관심사의 영역이지, 서비스영역에서는 로그인한 사용자인지 아닌지에 대해서 관심을 주지 않고, 단순히 사용자를 입력받아 처리한다~ 라고 관심사를 두고 서비스를 만들어야지, 좀 더 확장에 유연한 코드가 됩니다
Q3. 비즈니스 로직에 대한 추상화
리팩토링 관련 책중에 파이브 라인스 오브 코드(https://product.kyobobook.co.kr/detail/S000200661796) 라는 책이 있습니다.
이 책에서는 모든 메서드를 5줄 이하로 만들것을 권하고 있죠.
이게 의미하는바가 바로 추상화를 시켜서 메서드를 분리시키라는것 이에요.
이런 5줄 제한 규칙은 너무 극단적인 예시기 때문에 실제로 따르는것을 권하는것 보단 이와 같은 규칙을 하나 두고 리팩토링을 해보면 좋을것 같습니다.
제가 가지고 있는 한가지 규칙은, 이 로직을 모르는 사람이 봣을때 과연 이해할수 있을것인가? 인데요, 이 규칙을 지키기 위해선 상당히 많은 부분을 메서드로 추출해내고 + 적절한 메서드 이름을 줌으로써 이해를 돕게 해야만 합니다.
지금 이 주석 아래의 이부분도 마찬가지인데요, 다른사람이 보기에는 이 주석 없이는 무슨 행동을 하는것인지 이해 할수가 없을겁니다.
하지만 이 부분을 메서드로 추출하고 moveOtherFolder등의 이름을 준다면 주석 없이도 이해할수 있겟죠? 이런 수준의 추상화를 해보는건 어떨까요?
Beta Was this translation helpful? Give feedback.
All reactions