Skip to content
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

[Spring 체스 - 2단계] 레넌(조형래) 미션 제출합니다. #473

Merged
merged 33 commits into from
May 4, 2022

Conversation

brorae
Copy link

@brorae brorae commented Apr 30, 2022

안녕하세요, 스티치!
2단계도 리뷰 잘 부탁드리겠습니다!
1단계에서 피드백해주신 부분들을 통해 모두 생성자 주입 방식으로 의존성을 주입하도록 수정하였습니다. 그리고 테스트 코드 내에서 스키마를 실행하는 것을 schema.sql을 따로 만들어 실행해주도록 수정하였습니다.
감사합니다.

Copy link

@lxxjn0 lxxjn0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

레넌, 2단계 미션 잘 봤습니다 👍🏼

앞서 전달드린 피드백들은 잘 반영해주셨네요!! 이번에 조금 더 확인해보면 좋을 점들에 대해서 간단히 피드백 남겨보았습니다!! 한번 확인하고 반영해주세요 :)

src/main/java/chess/controller/WebController.java Outdated Show resolved Hide resolved
src/main/java/chess/controller/WebController.java Outdated Show resolved Hide resolved
src/main/java/chess/controller/WebController.java Outdated Show resolved Hide resolved
src/main/java/chess/controller/WebController.java Outdated Show resolved Hide resolved
src/main/java/chess/dao/FakeBoardDao.java Outdated Show resolved Hide resolved
src/main/java/chess/dto/BoardDto.java Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
Copy link

@lxxjn0 lxxjn0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

레넌, 피드백 반영한 부분 잘 봤습니다!! 코드가 훨씬 더 깔끔해진 것 같네요 :)

이번에는 기존 코드에서 개선해볼만한 부분에 대해서 리뷰를 조금 달아놨어요!! 가장 중점적으로 봐야 할 부분은 기존 console 중심적인 로직에 대한 개선입니다!! 제가 리뷰를 달아둔 부분은 일부분이지만 전체적으로 한번 검토해보고 개선해보면 좋을 것 같아요 (직접 문자열 입력을 줘서 로직을 실행하는 부분 등)

그리고 매직 넘버들도 많이 보이는데 그런 부분들도 한번 체크해서 개선해주면 좋을 것 같아요 👍🏼

src/main/java/chess/domain/state/State.java Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
src/main/java/chess/dao/GameDao.java Outdated Show resolved Hide resolved
src/main/java/chess/dao/BoardDaoImpl.java Show resolved Hide resolved
src/main/java/chess/dao/GameDaoImpl.java Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
src/main/resources/application.yml Outdated Show resolved Hide resolved
src/main/java/chess/service/ChessService.java Outdated Show resolved Hide resolved
Copy link

@lxxjn0 lxxjn0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

레넌, 피드백 반영하느라 수고 많으셨습니다!!

처음 작성한 코드보다 훨씬 더 발전한 코드로 마무리 지어주신 것 같네요 :) 그간 미션 진행하시느라 수고 많으셨습니다 💯

다음에 또 만나요 😄

@lxxjn0 lxxjn0 merged commit d0e939a into woowacourse:brorae May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants