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

[4, 5단계 - 체스] 다니(이다은) 미션 제출합니다. #258

Merged
merged 38 commits into from
Apr 11, 2021

Conversation

da-nyee
Copy link

@da-nyee da-nyee commented Apr 4, 2021

안녕하세요, 휴! 다니입니다 🙌

체스 미션.. 정말 어렵네요ㅠㅠ
생각보다 PR이 더 늦어졌습니다 😥

이번 단계에서는 필수 요구사항을 중점적으로 구현했습니다!
선택 요구사항도 구현할까 고민했지만, 선택까지 구현하면 PR이 더 늦어질 것 같아 필수까지 완료하고 PR을 보냅니다!

DB DDL은 README.md에 정리해두었어요.
테이블을 생성할 때 이 부분을 참고해주세요!

데이터 INSERT에 관한 DML은 README에 따로 적어두지 않았어요.
따라서, 초기 테이블에 데이터를 세팅하기 위해 프로그램 실행 전 initializePieceStatus(), initializeTurn()을 먼저 실행해주세요!

미션을 진행하면서 궁금한 점이 몇 가지 생겼는데요.
질문 드려도 괜찮을까요?!

궁금한 점 1.

프로그램에 DB를 추가하며 파일 구조에 대해 고민하게 됐어요.
MVC 패턴을 유지하면서 DB 로직을 추가하고 싶은데, 이를 어디에 두어야 할 지 궁금했는데요.
주변 크루들에게 물어보니 Repository Layer에 두면 된다고 해서 이곳에 DB 로직을 작성해뒀어요.
그런데, 다른 크루들의 코드를 참고하니 Service Layer도 있더라구요!
현재 제 코드는 Controller - Repository - DB 와 같은 구조를 가지고 있어요.
여기에 Controller - Repository 사이에 Service도 추가하는 게 좋을까요?
만약 Service가 있다면 Controller의 책임이 적어지고 구조가 조금 더 잘 나뉘어진 느낌이 들 것 같긴 해요!
하지만, 지금 단계에서 필요한 내용인지 궁금합니다!

궁금한 점 2.

DB Connection은 언제 종료해야 하나요?
서버가 돌아가는 동안에는 계속 DB가 유지되는 게 맞지 않나 싶어서 지금은 따로 종료하고 있지 않아요.
한편으로는, 그래도 DB 연결을 끊는 게 필요할 거 같아요.
DB 연결을 종료하는 적절한 시점이 언제인지 궁금합니다!

궁금한 점 3.

테스트는 랜덤으로 실행되지만, ChessDaoTest는 순차적으로 실행시키고 싶어 @TestMethodOrder을 추가했어요.
그런데, 테스트는 순서에 의존하면 안 된다는 생각이 들어요. 이런 방식으로 순서를 강제시켜도 괜찮은가요?

제 질문은 여기까지입니다!

이번 코드 리뷰도 잘 부탁드립니다 ✨
매번 귀한 시간 내주셔서 정말 감사합니다 🙇‍♀️❣

- piece_status, turn 테이블 생성
- piece_status, turn 테이블의 레코드를 초기화하는 Create 구현
- piece_status, turn 테이블의 레코드를 전부 삭제하는 Delete 구현
- removeAllPieces, removeTurn 테스트 작성
- piece_status 테이블의 레코드를 모두 읽어오는 Read 구현
- 기물이 이동하면 source -> target으로 piece_position이 변경되는 Update 구현
- 기물이 이동하면 target 위치의 기물이 삭제되는 Delete 구현
- 현재 white 턴인지 black 턴인지 읽어오는 Read 구현
- 기물이 이동하면 white <-> black으로 턴이 변경되는 Update 구현
- 대각선에 상대 기물이 있는 경우에만 이동 가능하도록 전략 수정
Copy link

@Hue9010 Hue9010 left a comment

Choose a reason for hiding this comment

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

다니!! 웹 적용이 어려웠을 텐데 잘 적용해 주셨네요! 👍👍👍
질문에 대한 답변과 몇가지 피드백 남겼는데 확인하고 궁금한건 언제든 질문주세요!!
혹시나 제가 빠트린 질문이 있다면 말씀주세요!!

Comment on lines +38 to +45
public void closeConnection(Connection con) {
try {
if (con != null)
con.close();
} catch (SQLException e) {
System.err.println("con 오류:" + e.getMessage());
}
}
Copy link

Choose a reason for hiding this comment

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

DB Connection은 언제 종료해야 하나요?
서버가 돌아가는 동안에는 계속 DB가 유지되는 게 맞지 않나 싶어서 지금은 따로 종료하고 있지 않아요.
한편으로는, 그래도 DB 연결을 끊는 게 필요할 거 같아요.
DB 연결을 종료하는 적절한 시점이 언제인지 궁금합니다!

답변에 앞서 혹시 아래 에러가 발생한 적 없나요? 🤔

Data source rejected establishment of connection, message from server: "Too many connections"

답변을 드리자면 거의 맞추셨네요! 👍 커넥션이 생각보다 가벼운 자원이 아니랍니다. 그러다보니 한번 연결한걸 재활용하죠. 이를 어떻게 재활용하느냐 하면 자카르타 DBCP API를 이용한 커넥션 풀 사용 의 처음 부분의 커넥션 풀이란 정도만 보셔도 충분합니다.
풀, pool 어디서 들어 보셨을 수도 있을 겁니다. 쓰레드 풀 등등이 있습니다. 대부분 같은 이유로 풀을 만들어 사용한답니다.
그래서 커넥션을 맺은 다음 계속 재활용 한다는 점에서는 맞추셨습니다. 약간 고려치 못한 부분은 지금은 풀을 사용하는게 아닌 DB 접속 때마다 새로운 커넥션을 맺기때문에 무한히 늘어 납니다. 하지만 컴퓨터 자원은 무한하지 못하기때문에 어느순간 문제가 발생하겠죠? 그래서 제가 위의 메시지를 만나게 됐네요. 😢
자원을 다 사용하기 전에 종료했다면 위와같은 메시지를 볼 일은 없으셨을 겁니다. 혹은 디폴트 설정에 따라 일정 시간 사용하지 않는 커넥션이 종료됐을 수도 있습니다.(이건 확실치 않기 때문에 알아봐야 하지만 지금은 중요하지 않다고 보네요.)

Copy link
Author

Choose a reason for hiding this comment

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

질문에 대한 답변 감사합니다! 🙇‍♀️ 커넥션 풀에 대해 알게 되었어요 👍
자원 해제는 try-with-resources 문법으로 해결했습니다!
43c22b7 여기서 확인하실 수 있어요 !

Copy link

Choose a reason for hiding this comment

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

Java - Try-with-resources로 자원 쉽게 해제하기

try-with-resources는 try(...)에서 선언된 객체들에 대해서 try가 종료될 때 자동으로 자원을 해제해주는 기능입니다. try에서 선언된 객체가 AutoCloseable을 구현하였다면 Java는 try구문이 종료될 때 객체의 close() 메소드를 호출해 줍니다.

제가 알기로도 선언된 객체만 정리해주는 걸로 알고 있어요. 그래서 PreparedStatement는 종료되지만 Connection은 닫히지 않고 있는걸로 보여요.

다시하기를 계속 눌러보세요. "Too many connections"을 보실 수 있을겁니다. 😋
(다만 설정이나 장비 스펙에 따라 다를 수는 있겠네요.)

        try (Connection connection = getConnection();
             PreparedStatement psmt = connection.prepareStatement(query)) {

psmt.executeUpdate();
}

public void initializeTurn() throws SQLException {
Copy link

Choose a reason for hiding this comment

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

throws SQLException

워낙 다른 분들도 공통 되는 내용이라 큰틀에서 자가 복붙 합니다. 🙄

[Toby spring] 사라진 SQLException - 예외 처리 회피 마지막 문단

만약 DAO가 SQLException을 생각 없이 던져버리면 어떻게 될까? DAO를 사용하는 서비스 계층이나 웹 컨트롤러에서 과연 SQLException을 제대로 처리할 수 있을까? 아마도 이런 경우라면 DAO에서 던진 SQLException을 서비스 계층 메서드가 다시 던지고, 컨트롤러도 다시 던지도록 선언해서 예외는 그냥 서버로 전달되고 말것이다. 아마도 throws SQLException과 같이 구체적인 예외를 던지도록 선언하기가 귀찮아서 모든 예외를 생각없이 던져버리게 하는 throws Exception을 사용할 가능성이 높다.

이런글을 보면 내 의식의 흐름을 어떻게 알고 이렇게 콕 찝었나 찔리죠. 😋

아마 위 포스팅을 봐도 이해하기 힘든 내용이 많을 겁니다. 템플릿 등등 쉽지 않은 내용에다가 연속된 내용을 다루는 책의 일부분이기 때문입니다. 그래도 좋은 내용이라 이해안되는건 이해 안되는데로 예외 관련만 가볍게(?) 봐도 좋을 것 같아요.
다만 이해 안돼도 지금은 그냥 넘어가고 이런 내용이 있었다는 것만 참고해뒀다가 나중에 비슷한 상황이 다시 생겼을 때 보면 지금보다는 그때 많은 도움이 될 겁니다. 😉

결론만 말씀드려보자면 위 글에서는 예외 변환을 추천하지만 지금 단계에서는 프레임워크 단에서의 예외 처리를 다루지 않기 때문에 예외 변환이 아닌 stacktrace 혹은 메시지를 출력(DAO에서) 정도로 끝내는 걸 추천드립니다. 정확히는 지금은 예외 변환을 적용하는걸 추천드리지 않습니다.

Comment on lines 157 to 158
CHESS_REPOSITORY.initializePieceStatus(filteredChessBoard);
CHESS_REPOSITORY.initializeTurn();
Copy link

Choose a reason for hiding this comment

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

여담이지만

따라서, 초기 테이블에 데이터를 세팅하기 위해 프로그램 실행 전 initializePieceStatus(), initializeTurn()을 먼저 실행해주세요!

/reset을 호출하는걸로 해결 했네요. 😄

생각해보니 위 방법을 도입(?)하여 새로하기 기능을 만들 수 있지 않을까요? 👀

public class WebUIChessController {
public static final Gson GSON = new Gson();
public static final Command START = CommandFactory.initialCommand("start");
public static final ChessRepositoryImpl CHESS_REPOSITORY = new ChessRepositoryImpl();
Copy link

Choose a reason for hiding this comment

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

프로그램에 DB를 추가하며 파일 구조에 대해 고민하게 됐어요.
MVC 패턴을 유지하면서 DB 로직을 추가하고 싶은데, 이를 어디에 두어야 할 지 궁금했는데요.
주변 크루들에게 물어보니 Repository Layer에 두면 된다고 해서 이곳에 DB 로직을 작성해뒀어요.
그런데, 다른 크루들의 코드를 참고하니 Service Layer도 있더라구요!
현재 제 코드는 Controller - Repository - DB 와 같은 구조를 가지고 있어요.
여기에 Controller - Repository 사이에 Service도 추가하는 게 좋을까요?
만약 Service가 있다면 Controller의 책임이 적어지고 구조가 조금 더 잘 나뉘어진 느낌이 들 것 같긴 해요!
하지만, 지금 단계에서 필요한 내용인지 궁금합니다!

말씀주신 것 처럼 이번 단계는 콘솔용으로 만든 애플리케이션을 웹 애플리케이션으로 옮겨보는 경험과 DB를 사용하는 게 목적이지 여기에 있어 Service 등등 일부 개념들을 적용하는데 있다고 생각치 않습니다. 제가 코치는 아니기 때문에 확신할 수 있는건 아니지만 과제의 요구사항과 과정상에서 해당 내용을 배우지 않는다는 점과 가장 중요하게는 리뷰어들이 익숙한 스프링이 아닌 스파크로 진행하신 점을 봤을 때 그렇습니다. 웹 적용을 스프링보다는 좀더 가볍게(?) 진행해보고자 스파크로 선택하신 걸로 보여요.

여러모로 스프링을 본격적으로 시작하실 때 배울 내용이기때문에 지금 단계서 해야할 필요는 없다고 생각합니다.

Copy link

Choose a reason for hiding this comment

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

그래도 몇가지 얘기해보고 싶은 부분이 있는데 ChessRepository를 인터페이스로 만든 이유가 무엇인가요?ㅎㅎ

관련 내용으로는 아래 글들을 보면 좋을 것 같아요. 꼭 정답이라는건 아니니 우선은 참고만 해도 좋습니다. 😉
Business Layer에 Interface를 만들어야 할까?
Spring 에서 Service 인터페이스 사용?

Copy link
Author

Choose a reason for hiding this comment

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

프로그램 파일 구조를 고민할 때 다른 크루들의 코드를 참고했어요!
한 크루가 Repository 인터페이스를 만들고, 이를 RepositoryImpl 클래스에서 구현했더라구요.
그래서 이렇게 하는 거구나 싶어서 저도 ChessRepositoryChessRepositoryImpl을 만들었습니다...!

그런데, 휴가 남겨주신 두 링크들을 읽어보니 무작정 인터페이스로 추상화 시키는 게 옳은 건 아닌 거 같아요!
휴는 보통 어떻게 하시나요?? 인터페이스를 먼저 만드시나요, 아니면 클래스를 만들고 필요에 따라 추상화 하시나요?

Copy link

Choose a reason for hiding this comment

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

스프링의 Service를 인터페이스화 하는 건 아직 저도 그게 필요했던 상황이 없어서 크게 고려치 않고 있어요.ㅎㅎ 그게 필요할 것 같은 상황이면 고려를 할 것 같아요.
다시 생각해보니 스프링에서 Repository는 인터페이스로 만들고 구현은 스프링이 해주는(?) 구조인데 이를 차용한 걸 수도 있겠네요. Repository 보다는 이후 스프링을 사용할 때 Service layer를 인터페이스화 시킬 필요가 있는지 정도로 봐주면 좋을 것 같아요.

스프링을 배제한 인터페이스로의 추상화 얘기라면 인터페이스가 필요한 상황인가를 먼저 생각해볼 것 같아요.

Comment on lines 18 to 24
public void initializePieceStatus(final Map<String, String> board) throws SQLException {
for (Map.Entry<String, String> boardStatus : board.entrySet()) {
PieceRequestDto pieceRequestDto = new PieceRequestDto(
boardStatus.getValue(), boardStatus.getKey());
chessDao.initializePieceStatus(pieceRequestDto);
}
}
Copy link

Choose a reason for hiding this comment

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

사실 약간 걱정했던 부분인데 dto 관련 정보에 dto를 layer가 바뀔 때마다 사용해야 한다는 내용과 Service <-> repository(or dao)간에도 사용해야 한다는 글들이 많아 살짝 걱정했는데 적용하셨군요. 😢

어떠셨나요? dto가 있어 더 편하셨나요 아니면 거꾸로 지금과 같은 변환 작업으로 번거로우셨나요? 그래서 개인적으로는 필요하지 않다면 Request, Response를 제외하고는 dto를 만들지 않는 편이랍니다. 그러긴해도 db의 table과 클래스간 차이가 있다면 필요에 의해 만들기도 한답니다. 하지만 꼭 필요하지 않은데 layer 간 통신이라는 이유로 모두 적용하는건 다른걸 다 떠나서 작업자 입장에서 너무 번거로운 작업이 많이 생긴다고 봅니다.

혹시 DTO(VO) 작성하시나요?
(참고로 DTO와 VO는 같지 않습니다.)

Copy link
Author

Choose a reason for hiding this comment

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

앗 DTO를 무조건 사용해야 하는 줄 알고 적용했는데 아니었군요...!!
이번 같은 경우에는 DTO로 변환하는 과정이 필요해서 약간 번거롭다고 느꼈어요.
리팩토링 하면서 따로 변환하지 않고 값을 그대로 넘겼는데, 이 방식이 조금 더 편리한 것 같아요!
휴의 말대로 DTO는 필요에 따라 만드는 게 좋을 거 같아요 !

whitePieces.add(chessBoardEntry.getValue());
}

CHESS_REPOSITORY.initializePieceStatus(stringChessBoard);
Copy link

Choose a reason for hiding this comment

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

            CHESS_REPOSITORY.removeAllPieces();

Line 46과 함께 쓰이는데 매번 지우고 다시 넣을 필요가 있나요? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

현재 프로그램이 동기로 작동해서 매번 지우지 않고 초기화하면 계속 레코드가 추가되더라구요..!
그래서 get 요청이 올 때마다 전체 레코드를 삭제하고 새로 추가하게 구현했습니다!


if (!(whitePlayer.getPieces().isKing() && blackPlayer.getPieces().isKing())) {
round.changeToEnd();
}
Copy link

Choose a reason for hiding this comment

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

제가 잘못 파악한게 아니라면 대략 여기까지가 round를 세팅하는 작업인 걸로 보이네요. 이를 봤을 때는 두가지를 생각 해 볼 수 있을 것 같아요. 아래의 move까지 봤을 때 piece_status, current_turn 테이블에서 가져온 데이터로 round 내부적으로 초기화 하는 기능을 추가 할 수 있지 않을까?

지금 체스의 문제 중 하나가 WebUIChessController가 round를 상태로서 관리하고 있다는 겁니다. 즉, 여러사람이 동시에 플레이하면 하나의 round를 조작하게 되니 문제가 생길겁니다. 그러니 상태로 가지고 있지 말고 매 요청마다 piece_status, current_turn을 통해서 가져온 데이터로 round를 새로 만들면 어떨까요? 이렇게 되면 게임방도 구현 할 수 있을 것 같네요.

위 방법으로 round를 상태로 가지지 않을 수 있을 것 같다고 보긴했지만 제가 100% 이해하지 못한 상태서 답변 드린거라 무조건 바꿀 수 있는건 아닙니다. 그점은 참고 바래요. 🙏

Copy link
Author

Choose a reason for hiding this comment

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

/chess에서 처음에 DB로부터 pieces 정보를 가져오고, 이를 whitePieces와 blackPieces에 담아 새로운 round를 만들어주고 있어요!
이렇게 하는 방식이 piece 테이블에서 가져온 데이터로 매번 round를 새로 생성하는 게 아닌가요...?!
이 피드백을 반영하고 싶은데, 어떻게 해야할 지 감이 잘 안 와서 한번 여쭤봅니다!

Copy link

Choose a reason for hiding this comment

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

이렇게 하는 방식이 piece 테이블에서 가져온 데이터로 매번 round를 새로 생성하는 게 아닌가요...?!

넵, 말씀주신 것 처럼 round를 새로 생성하는게 맞습니다. 말이 길어지기도 해서 더 어려워졌는데

/chess의 수정을 하는게 아닌 WebUIChessController의 round 필드를 제거하는게 목접입니다!
/move에서도 /chess와 같이 pieces에서 가져온 값 기반으로 round를 사용하게 바꾸면 round를 WebUIChessController의 상태로 관리하지 않을 수 있어 보여 드린 피드백이였답니다.(reset, start도 round를 참조하고 있읜 좀더 고려해봐야 할 겁니다.)

Controller가 상태를 가지지 않게 하는 경험을 해보면 좋을 것 같아 가능하다면 개선해보면 좋을 것 같아요. 다만 선택 사항으로 꼭 작업하실 필요는 없어요. 그렇기 때문에 답변에 따라서는 이번 단계는 여기서 마무리 할게요. 😉

혹시 이해 안되시는 부분은 바로 말씀주세요!

Copy link
Author

Choose a reason for hiding this comment

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

휴!! 이 피드백을 반영해보려고 코드를 이리저리 고쳐봤는데, Round의 결합도가 높아서 쉽지 않네요 😥

말씀하신 대로 Controller가 상태를 가지지 않게 하려고 각 요청 내에서 Round를 매번 새로 생성하게 시도해봤는데요.

특히, /chess에서 아래와 같이 round를 계속 새로 생성하니까 move 이후에도 round 내의 turn이 따로 변경되지 않아 무조건 하얀색 기물만 움직일 수 있고, 실제 객체 내의 값과 DB 값이 불일치하는 현상이 발생합니다 ㅠ

Round round = new Round(StateFactory.initialization(PiecesFactory.whitePieces()),
                StateFactory.initialization(PiecesFactory.blackPieces()),
                CommandFactory.initialCommand("start"));

round = new Round(StateFactory.initialization(new Pieces(whitePieces)),
                    StateFactory.initialization(new Pieces(blackPieces)),
                    CommandFactory.initialCommand("start"));

피드백을 반영해보려고 Round에 대해 고민하면서 객체간 결합도가 크면 리팩토링 하기 어렵다는 것을 알게 되었어요.

지금까지 객체의 결합도는 낮게, 응집도는 크게 설계하라는 내용을 머리로는 이해했지만 크게 와닿지는 않았는데요.
이번 미션을 하면서 이 내용이 왜 중요한지 직접 느껴볼 수 있었던 것 같아요.

제 코드의 경우에는 Round의 결합도가 크니까 콘솔을 웹으로 구현하면서 round의 상태를 변경하기 위해 추가해야 되는 코드가 많아지더라구요,,

객체의 결합도가 낮을수록 객체는 자유로워지니까 어떤 상황에서도 대처하기 쉬울 것 같아요.

아무튼,, 이 피드백을 반영하지 못해 너무 아쉽습니다.. 💦

Copy link

Choose a reason for hiding this comment

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

이건 제가 웹 적용 전 단계에서 미리 고려해서 같이 개선을 했어야 했는데 놓친것도 크네요. 😭 🙏
다음 단계부터는 웹을 고려한 환경에서 구현을 하기 때문에 이번과 같은 상황이 발생하진 않을거에요. 그런 점에서 개선을 하는 것보다 개선을 해보려다가 실패하고 무엇때문에 실패했고, 어떤게 중요한지 알게 되는게 더 큰 학습이 된다고 생각해요.

마찬가지로 현업에서도 장애를 통해 학습한다는 말도 있거든요. 더 보편적으로는 실패를 통해 성장한다고 하잖아요. 실패를 했던 경험 덕분에 그에 대한 사항은 좀더 확실히 기억하게 되죠. 특히나 (개인적으로 생각하기로)단계를 진행하면서 무언가 학습하는게 중요하지 단순히 단계를 완료하는게 중요하지 않다고 봅니다. 그런 점에서 아쉬운게 아닌 더 큰 경험을 했다고 생각해요. 👍

여담이지만 저는 예전(우테코 과정은 아니지만)에 체스에 db 적용하는 것에서 실패 했었습니다. 😆 그리고 이번 단계서 위 제안이 필수 사항도 아니라서 이미 충분히 잘해 주셨습니다. 👍👍

Comment on lines 120 to 140
@Order(10)
@DisplayName("piece_status 테이블의 모든 레코드를 삭제한다.")
@Test
public void removeAllPieces() throws Exception {
chessDao.removeAllPieces();
}

@Order(11)
@DisplayName("turn 테이블의 레코드를 삭제한다.")
@Test
public void removeTurn() throws Exception {
chessDao.removeTurn();
}

@Order(12)
@DisplayName("target 위치의 기물을 삭제한다.")
@Test
public void removePiece() throws Exception {
MoveRequestDto moveRequestDto = new MoveRequestDto("a6", "a7");
chessDao.removePiece(moveRequestDto);
}
Copy link

Choose a reason for hiding this comment

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

궁금한 점 3.
테스트는 랜덤으로 실행되지만, ChessDaoTest는 순차적으로 실행시키고 싶어 @TestMethodOrder을 추가했어요.
그런데, 테스트는 순서에 의존하면 안 된다는 생각이 들어요. 이런 방식으로 순서를 강제시켜도 괜찮은가요?

dao까지 꼼꼼히 테스트 해주셨네요. 👍
각 테스트가 독립적인게 좋기 때문에 말씀처럼 순서에 의존하지 않는게 좋답니다. 하지만 TestMethodOrder가 왜 존재할까요?? 이상적인건 모든 테스트가 독립적인게 좋긴해도 필요하면 사용하라고 제공을 하는겁니다. 😉
이걸 엄격히 지킬지는 개인 혹은 팀에 따라 다를 것 같아요. 저는 대체적으로 뭐든 지향하는 편이라 엄격하게 적용하는 것보다 필요에 따라서는 이와 같은 방법도 적용하는 편입니다.

다만 이 곳에 피드백을 남기는 이유가 있는데 순서를 만든 이유가 removeXXX들 때문이 맞을까요? 그렇다면 이는 스프링 기준으로는 아래와 같은 방법으로 처리 가능합니다.

트랜잭션
@test 어노테이션과 @transactional 어노테이션을 함꼐 사용했을 경우 테스트가 끝나면 rollback 되게 처리하면 됩니다.
[spring] spring boot test

하지만 지금은 Spark이니 그에 맞는 방법이 있겠지만 이번만 사용하고 말 프레임워크를 위해 이 이상의 시간을 쓰는건 불필요하다고 봐요. 😉

Copy link
Author

Choose a reason for hiding this comment

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

맞아요! removeXXX 메소드들 때문에 순서를 지정해줬습니다!
아하 스프링에서는 @test@transactional을 함께 사용해서 처리가 가능하군요 😮
다음 레벨에서 테스트할 때 순서가 중요한 경우에 해당 어노테이션들을 활용해봐야겠습니다!

docs/README.md Outdated
Comment on lines 100 to 102
SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0;
SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0;
SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION';
Copy link

Choose a reason for hiding this comment

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

마지막 라인까지 포함해서 어떤 목적으로 작성한걸까요?(최종적으론 기존 값으로 돌려 놓는 것 같지만ㅎㅎ)
가능하면 db 설정은 직접 만지지 않는게 좋습니다. 왜냐면 어렵거든요. 😳 는 반쯤 농담이지만 남들이 모르는 설정으로 인해 나중에 혹시나 설정으로 인해 문제가 생겼을 때 원인을 알기 힘들답니다.

특정 목적을 위해 이렇게 바꾸고 한다는건 결국 이러한 설정을 모르는 사람은 이 db 혹은 아래 테이블을 제대로 볼 수 없다는 얘기가 됩니다.

Copy link
Author

Choose a reason for hiding this comment

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

Workbench에서 ERD를 그린 다음 Forward Engineer로 DDL을 추출했는데, 이 과정에서 작성된 내용들입니다!
저도 이게 어떤 설정인지 제대로 모른 채 그저 옮겨 적었는데요...!
무엇인지 확실히 알고 적었어야 했는데 그러질 않았네요.. 😥 바로 삭제했습니다!

Copy link

Choose a reason for hiding this comment

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

아닙니다. 복붙하는 과정에서 지금처럼 고려치 않은 값들이 같이 복사 될 수가 있는데 보통은 문제가 없겠지만 이 값들로 인해 생각치 못한 사이드 이펙트가 발생할 수도 있습니다. 그래서 (특히나 학습 단계면) 그냥 유의만 해주시면 됩니다!
의도하신건지 궁금했던 겁니다. 😉

-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `mydb`.`piece_status` (
`id` BIGINT NOT NULL AUTO_INCREMENT,
`piece_name` VARCHAR(45) NOT NULL,
Copy link

Choose a reason for hiding this comment

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

모든 분께 드리고 있는 퀴즈인데 MySQL에서 VARCHAR(45)면 한글은 몇글자까지 저장 가능할까요? 😏
엄청 중요한 퀴즈는 아니기 때문에 생각나는 답변을 주셔도 충분해요!

Copy link
Author

Choose a reason for hiding this comment

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

DB 테이블에 직접 값을 INSERT 해서 확인해봤어요!

INSERT INTO turn (current_turn) VALUES('가나다');

SELECT current_turn, LENGTH(current_turn) AS len
FROM turn
WHERE id=16;

결과는 아래와 같이 나왔습니다!

korean_varchar

이를 통해 한글은 1글자당 3byte인 것을 알 수 있습니다.
따라서, MySQL에서 VARCHAR(45)면 한글은 총 45 / 3 == 15글자까지 저장이 가능합니다!

Copy link

Choose a reason for hiding this comment

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

퀴즈를 낸 이유가 바이트로 생각하기 쉬워서였습니다!! 😄

과거버전(4.1 이전)을 사용하는게 아닌 이상 글자수랍니다. 아래 포스팅처럼 간단히 테스트해 보실수도 있을겁니다. 😉
[MySql] varchar 5라면 한글은 몇 글자까지 들어갈 수 있을까?

mySql varchar 타입 질문이 있습니다.

MySQL 얘기이기 때문에 Oracle, MsSQL등 은 다를 수 있는 점은 유의하셔야 합니다.

사실 크게 중요하지도 대단한 내용이 아닙니다. 맡는 도메인에 따라 한글을 아예 안다룰수도 있고요. 마찬가지로 시니어 분도 한글을 다룰 일 이 없었다면 인지하지 못하고 있을 수도 있습니다. 아무래도 모든 것에 관심을 가질 수는 없거든요.
이러한게 있었다는 정도만 알고 있다가 나중에 필요해지면 한글 저장이 바이트가 아니였던거 같은데? 하면서 그때 찾아보면 됩니다. 😉

@da-nyee
Copy link
Author

da-nyee commented Apr 10, 2021

안녕하세요, 휴! 다니입니다 🙌

꼼꼼한 피드백과 답변들 정말 감사드립니다 💘
피드백 반영하면서 많이 배우고 공부했어요!
MySQL 관련 퀴즈를 통해서도 새로운 지식을 얻을 수 있었습니다 👍

달아주신 코멘트들에 저도 몇몇 생각과 질문들을 남겼어요!
확인해주시면 감사하겠습니다~!

이번에도 코드 리뷰 잘 부탁합니다!
항상 귀한 시간 내주셔서 감사합니다 🙇‍♀️✨

Copy link

@Hue9010 Hue9010 left a comment

Choose a reason for hiding this comment

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

다니! 반영 잘 해주셨네요. 👍👍
마무리 해도 좋을 것 같지만 한가지 좀 더 개선했으면 좋을 것 같은 부분 있어서 의견남겼어요!
확인해주시고 답변 주시면 좋을 것 같아요. 😉

Comment on lines +38 to +45
public void closeConnection(Connection con) {
try {
if (con != null)
con.close();
} catch (SQLException e) {
System.err.println("con 오류:" + e.getMessage());
}
}
Copy link

Choose a reason for hiding this comment

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

Java - Try-with-resources로 자원 쉽게 해제하기

try-with-resources는 try(...)에서 선언된 객체들에 대해서 try가 종료될 때 자동으로 자원을 해제해주는 기능입니다. try에서 선언된 객체가 AutoCloseable을 구현하였다면 Java는 try구문이 종료될 때 객체의 close() 메소드를 호출해 줍니다.

제가 알기로도 선언된 객체만 정리해주는 걸로 알고 있어요. 그래서 PreparedStatement는 종료되지만 Connection은 닫히지 않고 있는걸로 보여요.

다시하기를 계속 눌러보세요. "Too many connections"을 보실 수 있을겁니다. 😋
(다만 설정이나 장비 스펙에 따라 다를 수는 있겠네요.)

        try (Connection connection = getConnection();
             PreparedStatement psmt = connection.prepareStatement(query)) {


if (!(whitePlayer.getPieces().isKing() && blackPlayer.getPieces().isKing())) {
round.changeToEnd();
}
Copy link

Choose a reason for hiding this comment

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

이렇게 하는 방식이 piece 테이블에서 가져온 데이터로 매번 round를 새로 생성하는 게 아닌가요...?!

넵, 말씀주신 것 처럼 round를 새로 생성하는게 맞습니다. 말이 길어지기도 해서 더 어려워졌는데

/chess의 수정을 하는게 아닌 WebUIChessController의 round 필드를 제거하는게 목접입니다!
/move에서도 /chess와 같이 pieces에서 가져온 값 기반으로 round를 사용하게 바꾸면 round를 WebUIChessController의 상태로 관리하지 않을 수 있어 보여 드린 피드백이였답니다.(reset, start도 round를 참조하고 있읜 좀더 고려해봐야 할 겁니다.)

Controller가 상태를 가지지 않게 하는 경험을 해보면 좋을 것 같아 가능하다면 개선해보면 좋을 것 같아요. 다만 선택 사항으로 꼭 작업하실 필요는 없어요. 그렇기 때문에 답변에 따라서는 이번 단계는 여기서 마무리 할게요. 😉

혹시 이해 안되시는 부분은 바로 말씀주세요!

</head>
<body>
<div id="main-div">
<h1 id="chess-game">✨ 체스 게임 ✨</h1>
Copy link

Choose a reason for hiding this comment

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

퍼블리싱 쪽이 변경사항이 바로바로 시각적으로 나타나서 은근 재밌죠. 😄

- Connection도 객체로 선언하여 try 문이 종료될 때 자동으로 자원 해제
@da-nyee
Copy link
Author

da-nyee commented Apr 11, 2021

안녕하세요 휴!! 다니입니다 🙌

try-with-resources 피드백을 반영하고, round 피드백에 코멘트 남겼습니다!
확인해주시면 감사하겠습니다!

매번 빠른 코드 리뷰 감사합니다 🙇‍♀️💟

Copy link

@Hue9010 Hue9010 left a comment

Choose a reason for hiding this comment

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

다니! 빠르게 반영해주셨네요! 👍
이번 단계는 여기서 머지하도록 할게요. 웹 적용하시면서 고생 많으셨습니다!!
혹시 제가 놓친 질문이 있다면 알려주시고 그 외에도 궁금한건 언제든 질문 주세요. 😉
다음 단계도 화이팅입니다!!

public class PieceDao extends DBConnection {
public void initializePieceStatus(final String pieceName, final String piecePosition) {
String query = "INSERT INTO piece (piece_name, piece_position) VALUE (?, ?)";
try (PreparedStatement psmt = getConnection().prepareStatement(query)) {
Copy link

Choose a reason for hiding this comment

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

PieceDao 😏


if (!(whitePlayer.getPieces().isKing() && blackPlayer.getPieces().isKing())) {
round.changeToEnd();
}
Copy link

Choose a reason for hiding this comment

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

이건 제가 웹 적용 전 단계에서 미리 고려해서 같이 개선을 했어야 했는데 놓친것도 크네요. 😭 🙏
다음 단계부터는 웹을 고려한 환경에서 구현을 하기 때문에 이번과 같은 상황이 발생하진 않을거에요. 그런 점에서 개선을 하는 것보다 개선을 해보려다가 실패하고 무엇때문에 실패했고, 어떤게 중요한지 알게 되는게 더 큰 학습이 된다고 생각해요.

마찬가지로 현업에서도 장애를 통해 학습한다는 말도 있거든요. 더 보편적으로는 실패를 통해 성장한다고 하잖아요. 실패를 했던 경험 덕분에 그에 대한 사항은 좀더 확실히 기억하게 되죠. 특히나 (개인적으로 생각하기로)단계를 진행하면서 무언가 학습하는게 중요하지 단순히 단계를 완료하는게 중요하지 않다고 봅니다. 그런 점에서 아쉬운게 아닌 더 큰 경험을 했다고 생각해요. 👍

여담이지만 저는 예전(우테코 과정은 아니지만)에 체스에 db 적용하는 것에서 실패 했었습니다. 😆 그리고 이번 단계서 위 제안이 필수 사항도 아니라서 이미 충분히 잘해 주셨습니다. 👍👍

@Hue9010 Hue9010 merged commit 837d0cb into woowacourse:da-nyee Apr 11, 2021
@da-nyee
Copy link
Author

da-nyee commented Apr 15, 2021

📚 다니의 학습 로그

[Java] try-with-resources - 3

내용

링크

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.

None yet

2 participants