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

[라빈] - 체스 스프링 실습 레벨 1 제출합니다 #30

Merged
merged 6 commits into from
Apr 23, 2020

Conversation

giantim
Copy link

@giantim giantim commented Apr 22, 2020

작은곰 안녕하세요!

기존에 작성했던 코드에서 콘솔 관련된 부분을 삭제하고 리팩토링 후 제출합니다.

감사합니닿ㅎㅎㅎ

@giantim giantim marked this pull request as draft April 22, 2020 10:45
@giantim giantim marked this pull request as ready for review April 22, 2020 10:45
Copy link

@KimGyeong KimGyeong left a comment

Choose a reason for hiding this comment

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

안녕하세요! 작은곰입니다. 깔끔한 코드를 보면서 많이 반성하는 시간을 갖게 되었네요 😂
작은 피드백 2개 남겼습니다. 확인해서 변경해주시고 머지 받으시면 될 것 같습니다!
다음 단계도 기대하겠습니다~

post("/destination", this::move, json());
}

private String index(Request req, Response res) {

Choose a reason for hiding this comment

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

메서드의 컨벤션은 동사라는 피드백을 받은 적이 있습니다. 변경해보는 것은 어떨까요?

try (PreparedStatement pstmt = connectionManager.getConnection().prepareStatement(query); ResultSet rs = pstmt.executeQuery()) {
public List<MovingPosition> selectAll() throws SQLException {
String query = "SELECT * FROM history";
try (PreparedStatement pstmt = connectionManager.getConnection().prepareStatement(query); ResultSet rs = pstmt.executeQuery()) {

Choose a reason for hiding this comment

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

; 뒤에서 줄바꿈이 이루어지면 가독성이 좋아지지 않을까요?

@woowahan-pjs woowahan-pjs merged commit 210200b into woowacourse:giantim Apr 23, 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.

4 participants