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

[엘리] 스프링 실습 3단계 제출합니다. #166

Merged
merged 29 commits into from
May 3, 2020

Conversation

YebinK
Copy link

@YebinK YebinK commented May 1, 2020

안녕하세요 라흐!
이번에도 잘 부탁드려요 :)

YebinK and others added 28 commits April 22, 2020 13:33
    - 폰 움직임 메소드 분리
    - README.md 작성
    - 불필요한 공백 제거
    - DAO 중복 제거
    - 체스 말 별 초기화 클래스 제거
    - 메소드 네이밍 수정
    - 상수 추출
    - 스파크와 스프링에서 DB 연동이 가능하도록 변경
    - Set<Comment> 에서 List<Comment>로 변경
    - 새 게임 시작하면 새 방 번호 생성된 후 기존 게임 요청으로 redirect
    - MoveCommandRepository 제거
    - chess_room 테이블과 move_command 테이블 연결 위한 칼럼 추가 (chess_room_key)
Copy link

@Hamliet Hamliet left a comment

Choose a reason for hiding this comment

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

안녕하세요 엘리! 몇가지 체크한 사항 공유드릴게요!

  1. SpringCommandsDaoTest가 남아있어서 전체 테스트를 돌려봤을때 2개가 fail이 뜨네요. 불필요한 테스트로 보이는데 삭제해보심이 어떨까요?
  2. 말을 이동할 수 없는 곳으로 이동하면 이동할 수 없습니다. -> undifined팀이 이겼습니다. 메시지가 뜨고 목록으로 이동하네요! 수정하셔야 할 것 같습니다.
  3. 현재 왕이 먹히면 방이 삭제되고 방 목록으로 돌아가는데, 종료버튼을 눌렀을때도 같은 동작을 하면 어떨까요? 현재 종료버튼을 누르면 종료페이지로는 넘어가는데 방 삭제는 안되고있네요. (종료 페이지에 목록으로 이동 버튼도 있으면 좋겠어요 ㅎㅎ)
  4. CommandRepository에서 deleteAll() 함수를 Override한 이유가 있을까요?
  5. ChessRunner에 사용하지 않는 함수 ClearBoard()가 있네요.

확인해보시고 필요한 부분 반영하시면 될 것 같습니다!
고생하셨습니다. :)

model.addAllAttributes(chessService.makeMoveResponse());
return "chessGame";
@PostMapping("/")
public String playing(@RequestParam("newRoomName") String newRoomName, Model model) {
Copy link

Choose a reason for hiding this comment

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

사용하지 않는 파라미터(model)가 있네요!

Copy link
Author

Choose a reason for hiding this comment

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

피드백 감사합니다!
급하게 제출하느라 미처 확인하지 못한 부분들이었어요!

3번같은 경우는 의도한 부분이라 버튼 이름을 '저장 후 종료하기'로 수정했습니다!

의견 감사해요 :)

@woowahan-pjs woowahan-pjs merged commit 38b61c2 into woowacourse:yebink May 3, 2020
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.

3 participants