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

[소니] 체스 - 스프링 실습 2단계 제출합니다. #71

Merged
merged 16 commits into from
Apr 29, 2020

Conversation

sonypark
Copy link

리뷰 부탁드립니다 :)

Copy link

@moonyoungCHAE moonyoungCHAE left a comment

Choose a reason for hiding this comment

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

안녕하세요 소니~
리뷰가 조금 늦어져 죄송합니다.
annotation을 적절하게 잘 사용하셨네요. 😀덕분에 저도 배워갑니다 :)
깔끔한 코드 잘 봤습니다~

return GSON.toJson(chessService.createChessRoom());
@GetMapping("/")
public String renderIndexPage() {
return "index";

Choose a reason for hiding this comment

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

index로 저장된 view는 별도의 rendering없이도 접속할 수 있습니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

그러네요. index 파일은 굳이 매핑하지 않아도 되는데 불필요한 코드를 추가했군요.
피득백 감사합니다!

this.chessService = chessService;
}

@GetMapping("/board/{id}")

Choose a reason for hiding this comment

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

객체 자체를 return하는 게 어떨까요?

RestController를 사용하면 ResponseBody를 return하게 되고 이 과정에서 객체를 json 형식으로 바꿔줍니다.

Copy link
Author

Choose a reason for hiding this comment

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

오.. 이건 몰랐네요. Spring Annotation에 대해 더 공부해야겠네요 😅
덕분에 Gson을 사용할 필요가 없어졌습니다. 코드가 훨씬 깔끔해졌네요!

}

@PostMapping(value = "/move/{id}")
public String movePiece(@PathVariable String id, @RequestBody Map<String, Double> data) throws SQLException {

Choose a reason for hiding this comment

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

Double 대신 Integer를 사용하면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

클라이언트에서 fetch로 json 형태로 서버에 요청 할 때 숫자를 Double 형으로 보내서 Double로 받았는데, 타미가 말한대로 Integer로 하니 자동 형변환이 되네요!
덕분에 intValue를 사용할 필요가 없어 코드가 더 깔끔해졌습니다.
리뷰 감사합니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

추가로 MovePositionDto를 만들어 처리하도록 수정했습니다 :)

@PostMapping(value = "/move/{id}")
    public ResponseDto movePiece(@PathVariable String id, @RequestBody MovePositionDto movePositionDto) {
        int chessGameId = Integer.parseInt(id);
        Position source = Position.of(movePositionDto.getSourceX(), movePositionDto.getSourceY());
        Position target = Position.of(movePositionDto.getTargetX(), movePositionDto.getTargetY());
        return chessService.movePiece(chessGameId, source, target);
    }

@@ -16,6 +18,7 @@
import wooteco.chess.dto.StatusDto;
import wooteco.chess.dto.TurnDto;

@Service
public class ChessService {
private static final ChessGameDao chessGameDao = new ChessGameDao();

Choose a reason for hiding this comment

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

ChessService와 ChessGameDAO가 강한 의존 관계를 보이는데,
ChessGameDAO를 Bean으로 등록하면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋은 의견이네요. Bean으로 등록하는걸로 수정했습니다 :)


@PostMapping(value = "/move/{id}")
public String movePiece(@PathVariable String id, @RequestBody Map<String, Double> data) throws SQLException {
int pieceId = Integer.parseInt(id);

Choose a reason for hiding this comment

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

pieceId -> chessGameId
변수명을 잘못 적으신 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

앗.. 이곳도 잘못 썼군요.😅 수정했습니다!

- 클라이언트에서 fetch로 json 형태로 서버에 요청 할 때 숫자를 Double 형으로 보내서 Double로 받았는데, Integer로 하니 자동 형변환이 된다.
- RestController를 사용하면 ResponseBody를 return하게 되고 이 과정에서 객체를 json 형식으로 바꿔준다.
- index로 저장된 view는 별도의 rendering없이도 접속할 수 있다.
@gracefulBrown gracefulBrown merged commit 299537d into woowacourse:sonypark Apr 29, 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