-
Notifications
You must be signed in to change notification settings - Fork 461
[2주차 스터디 제출용] #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[2주차 스터디 제출용] #149
Conversation
wugawuga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
승현님 코드를 볼때마다 리뷰를 남긴다기 보다 항상 배우고 공부하게 되어서 감사합니다
리뷰를 많이 남기지 못해 죄송해요ㅠㅠ 수고 많으셨습니다
| public void doProcess(Map<String, Object> model) { | ||
| MatchingCommand matchingCommand = getMatchingCommand(); | ||
| try { | ||
| MatchingService.checkPairMatchingAlreadyExists(matchingCommand.getCourse(), matchingCommand.getMission()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MatchingService. static 메소드들에 파라미터로 getCourse, getMission 으로 받아서 던져주는데
MatchingCommand 만 넘겨주고 Service 클래스에서 필요한 것들만 뽑아서 쓰는 방법은 어떻게 생각하시나요?
개인 취향인 것 같은데 저는 command 를 넘겨주고 Service 클래스에서 뽑아쓰는 방식을 선호해요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 저도 우가님 방식으로 하는 게 좋아보여요! 파라미터 개수도 줄어드니 더 좋겠네요 :) 감사합니다!
| public static boolean isExistingWith(Level level, String missionName) { | ||
| return missions | ||
| .stream() | ||
| .filter(mission -> mission.isLevel(level)) | ||
| .anyMatch(mission -> mission.isName(missionName)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떻게 매치해야할까 고민이 진짜 많았는데 한 수 배웁니다 🙇♂️
reddevilmidzy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5시간 동안 집중해서 구현 하는게 정말 자신과의 싸움 같은데 매번 놀라네요 ㅎㅎ
수고많으셨습니당!
| java { | ||
| toolchain { | ||
| languageVersion = JavaLanguageVersion.of(8) | ||
| languageVersion = JavaLanguageVersion.of(11) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 여기서 빌드 그래들을 수정하는거였군요!
List.of 문법 사용이 안돼서 애먹었었는데,,ㅎ 감사합니당
| public static void doMatch(Course course, Mission mission) { | ||
| List<String> crewNames = CrewRepository.findAllByCourse(course); | ||
| if (crewNames.size() < 2) { | ||
| throw new IllegalStateException(NOT_ENOUGH_CREWS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
크루의 사이즈가 2보다 작을 때는 애초에 매칭이 불가하기 때문에 잘못된 메소드 호출이라고 판단하고 IllegalArgumentException이 아닌 IllegalStateException을 사용하신건가요?
이 둘을 어느 때에 어떤 차이를 두고 사용해야 하는지 감이 안잡혀서 질문드립니다!
| @@ -0,0 +1,28 @@ | |||
| package pairmatching.system; | |||
|
|
|||
| import pairmatching.controller.*; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 자동으로 와일드카드가 만들어진거 같은데 자바 컨벤션에 따라 와일드카드는 지양하는것이 좋으니, IDE에서 자동으로 와일드카드 변경하는 기능을 수정해도 좋을거 같아요!
No description provided.