양홍주 미션 제출합니다. #161
양홍주 미션 제출합니다. #161
Conversation
String이 아닌 객체 사용
…를 출력 후 해당 부분부터 다시 입력을 받는다.
AnTaeho
left a comment
AnTaeho
left a comment
There was a problem hiding this comment.
전체적으로 읽어봤습니다!
말씀해주신 controller 가 무거워지는 문제에 대해서는 저도 그렇다고 생각됩니다.
우선 저라면 좀 더 비즈니스 로직을 service 계층에 넘겨주었을 것 같습니다!
그런데 그렇다고 필요없어 보이거나 역할을 너무 벗어나는 로직이 있는 것 같지는 않아서
이 과제 자체가 조금 복잡하지 않았나 생각됩니다.
그 외에도 간단하게 리뷰 남겼습니다! 한번 읽어봐주세요!
| } | ||
| } | ||
|
|
||
| private void processFunctionInput(Function function) { |
There was a problem hiding this comment.
여기서 분기처리를 해주는 것은 어떤가요?
if(function.is~()) {
~~~();
return;
}
이런식으로요!
지금 방식은 모든 메서드에 다 들어가야해서 가독성이 떨어질 것 같습니다!
| } | ||
|
|
||
| private void processPairReset(Function function) { | ||
| if (function.equals(PAIR_RESET)) { |
There was a problem hiding this comment.
취향 차이일수 있는데 저는 function에 직접 물어보는 것이 좋다고 생각합니다.
function.isReset() 이런 식으로요!
| private void processRematching(RematchingOption rematchingOption, Course course, CourseMission courseMission) { | ||
| if (rematchingOption.equals(YES)) { | ||
| processMatchingResult(course, courseMission); | ||
| } | ||
| } |
There was a problem hiding this comment.
아니오를 선택하면 코스,레벨,미션을 다시 고르도록 해야하는데,
다시 처음으로 돌아가는 것 같습니다!
| private void processNonDuplicatedResult(CourseMission courseMission, Optional<MatchingResult> nonDuplicatedResult) { | ||
| MatchingResult matchingResult = nonDuplicatedResult.get(); | ||
| outputView.outputPairMatchingResult(MatchingResultMapper.from(matchingResult)); | ||
| matchingService.save(courseMission, matchingResult); | ||
| } |
There was a problem hiding this comment.
Optional 처리를 service나 repository에서 하는 것이 좋을 것 같습니다!
정확히는 repository에서 Optional을 벗겨서 오는 것이 좋지 않을까 생각합니다!
근거로 JPA Repository의 getById 메서드를 들려고 했는데 Deprecated 됐네요...
그래도 .orElseThrow 같은걸 사용해서 컨트롤러에는 깔끔한 객체를 보여주는 것은 어떤가요?
코틀린처럼 Elvis 연산자가 있으면 좋을텐데 ㅠㅠㅠ
| private Function inputValidFunction() { | ||
| return InputUtil.retryOnInvalidInput(this::inputFunction, | ||
| errorMessage -> outputView.outputErrorMessage(INVALID_FUNCTION.getMessage())); | ||
| } | ||
|
|
||
| private Function inputFunction() { | ||
| return inputView.inputFunction(); | ||
| } |
There was a problem hiding this comment.
private Function inputValidFunction() {
return InputUtil.retryOnInvalidInput(inputView::inputFunction,
errorMessage -> outputView.outputErrorMessage(INVALID_FUNCTION.getMessage()));
}
진짜 큰 차이 아니긴 한데 이렇게 수정하는건 어떤가요?
| import java.util.List; | ||
| import java.util.Objects; | ||
|
|
||
| public class Pair { |
There was a problem hiding this comment.
아 이생각을 못했네요...
제 코드 보시면 아시겠지만 List<List<Crew>> 이거 진짜 보기 싫었는데
통채로 Pair 객체로 만드려니 그거도 별로여서 안했는데 List<Crew>를 Pair로 묶는게 맞겠네요
|
|
||
| public class MatchingController { | ||
|
|
||
| private final int REMATCHING_COUNT = 3; |
There was a problem hiding this comment.
이런 부분에 대해서 저도 고민해봤는데,
3회 체크를 하려면 어쩔수 없이 필드에 값이 존재해야할 것 같았습니다.
하지만 이런 값을을 최대한 배제하려 해서 사용하지 않았는데 이부분에 대해선 어떻게 생각하시나요?
There was a problem hiding this comment.
시간이 없어서 분리하지 못했는데..ㅎㅎ 시간 여유가 있었다면 따로 enum에서 관리하는 방법으로 리팩터링 했을 것입니다!
2 participants
[시간 제한 관련]
[적용하지 못한 것]
[저번 주 미션에 비해 더 신경쓴 부분]
[다음 주 미션에 적용하려 하는 부분]
[다음 미팅에서 함께 얘기해보고 싶은 부분]