-
Notifications
You must be signed in to change notification settings - Fork 744
[사다리 타기 - 4단계] 리뷰 요청 드립니다. #886
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
Conversation
…ntroller 내에 Generator에 대한 context를 갖고 있게끔하여 캡슐화](next-step#859 (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.
~Process, Resolver 등이 모두 제거되었네요 👍
이러한 Helper 류 클래스들이 제거되었을 때의 장점을 느껴보셨으면 좋겠습니다 ㅎㅎ
4단계는 얘기해주신 부분중 후자의 방향 (주어진 코드를 베이스로 TDD 방식으로 재구현) 으로 진행하는 것이 본래 해당 단계의 의도인데요! 두가지 코드 모두 이용가능한 방향으로 작업해주셔서 다시 처음부터 하실 필요는 없을 것 같습니다.
내용은 간단한 피드백을 남겼는데요, 클래스가 메소드를 구현하는 기준에 대해서 한번쯤 생각해보면 좋을 주제라 Request changes 로 남겼습니다. 마지막단계인 만큼 화이팅 이십니다!
return new LadderResult(result); | ||
} | ||
|
||
public static LadderResult rideLadder( |
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.
해당 로직은 LadderGameController 에서 알고 있는 내용이므로 Controller 에서 수행할 수 있지 않을까요~?
LadderResult 는 해당 로직의 결과물일뿐 클래스 내부에 해당 static 메소드가 선언되어야하는 근거가 약할 것 같습니다!
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.
말씀하신 내용을 이해한대로 정리해보면
Controller에서 만들어진 도메인들을 가지고 사다리 타기 로직
을 통해 산출한 결과 값을
LadderResult 라는 클래스에서 해당 로직을 관리하는 것보다 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.
현재로서는 사다리 게임 결과를 산출하는데 이용되는 데이터를 모두 Controller 에서 알고 있으므로 Controller 에서 수행해도 괜찮다고 생각해요!
다만, 이러한 방식은 프로젝트가 커질수록 말씀하신 것처럼 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.
마지막 피드백까지 잘 반영해주셨네요 👍
사다리게임 같이 요구사항이 복잡해보이는 프로젝트의 경우 로직을 어떤수준까지 분리하여 나눠야할지 고민이 되는데요, 각 단계를 진행하시며 조금이나마 이 고민에 대한 실마리를 얻어가셨다면 좋겠습니다 ㅎㅎ
4단계도 머지하도록 하겠습니다.
미션 진행하시느라 고생 많으셨습니다!
다시 한번 객체지향 설계는 캡슐화와, 낮은 결합도, 높은 응집도에 대한 기준이 되어야 한다는 것을 다시 한번 되뇌이게 되었습니다. |
안녕하세요 리뷰어님 !
결론부터 말씀드리면 주어진 코드를 라이브러리라 생각하고 기존 코드와 바꿔가면서 사용할 수 있도록 쓸 수 있도록 하는 것인지 아니면
제가 작성한 코드와는 별개로 새로 주어진 코드를 기반으로 작성하는 것인지 이해하지 못해서 기존 코드 로직, 신규 코드 로직 메서드 2개를 작성해 놓았습니다.
피드백 영상에서는
인터페이스를 통해 연결할 수 있는지 생각해볼 수 있지 않을까?
라는 말씀을 하셔서기존 코드도 얼추 신규 코드의 구성 처럼 바꾼 뒤에 인터페이스로 연결하여 객체 지향 코드를 작성해야 하는 것인지 궁금해서 먼저 PR 요청 드려봅니다.
감사합니다.