-
Notifications
You must be signed in to change notification settings - Fork 412
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, 4단계 - 체스] 제이(이재윤) 미션 제출합니다. #530
Changes from 34 commits
9c2396a
6cb22ba
832bd00
aeebb02
9b4442a
d2c2a6e
48824eb
11282d7
700da6f
2a3a624
b64a004
3f8d963
6ff9da9
330b809
3d77a16
051ee0e
063af9f
7a719bc
62577bf
86f0102
f222984
41ce6c7
0db7ea5
189dd1b
dcdbb80
5f87010
14ae3ed
028bbc7
72e6bf2
f5c2788
60513f8
0299e0c
9f591d6
454c09d
00c90d1
cea5745
b8c9fd9
4eb9c8d
b0f58da
6a23cba
5a144d4
6b0a221
5c6b8fc
262eb61
1073412
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,66 +1,142 @@ | ||
package chess.controller; | ||
|
||
import chess.domain.commnad.Command; | ||
import chess.domain.commnad.LoadGameCommand; | ||
import chess.domain.game.ChessGame; | ||
import chess.factory.BoardFactory; | ||
import chess.service.BoardService; | ||
import chess.view.InputView; | ||
import chess.view.OutputView; | ||
import java.util.List; | ||
import chess.view.ResultView; | ||
|
||
public class ChessGameController { | ||
|
||
private static final String START_COMMAND = "start"; | ||
private static final String MOVE_COMMAND = "move"; | ||
private static final String END_COMMAND = "end"; | ||
private static final int COMMAND_INDEX = 0; | ||
private static final int SELECTED_PIECE = 1; | ||
private static final int DESTINATION = 2; | ||
private static final int BOARD_ID = 1; | ||
|
||
private final InputView inputView; | ||
private final OutputView outputView; | ||
private final ResultView resultView; | ||
private final BoardService boardService; | ||
|
||
public ChessGameController(final InputView inputView, final OutputView outputView) { | ||
public ChessGameController(final InputView inputView, | ||
final OutputView outputView, | ||
final ResultView resultView, | ||
final BoardService boardService) { | ||
this.inputView = inputView; | ||
this.outputView = outputView; | ||
this.resultView = resultView; | ||
this.boardService = boardService; | ||
} | ||
|
||
public void run() { | ||
ChessGame chessGame = new ChessGame(BoardFactory.createBoard()); | ||
LoadGameCommand loadGameCommand = inputView.readStatusOfGame(); | ||
ChessGame chessGame = loadGame(loadGameCommand); | ||
|
||
outputView.printStartMessage(); | ||
playChess(chessGame); | ||
|
||
List<String> inputCommand = inputView.readGameCommand(); | ||
resultView.printGameEnd(); | ||
} | ||
|
||
private ChessGame loadGame(final LoadGameCommand loadCommand) { | ||
if (loadCommand.isSavedGame()) { | ||
ChessGame chessGame = new ChessGame(boardService.findById(BOARD_ID), boardService.isLowerTeamTurnByBoardId(BOARD_ID)); | ||
outputView.printBoard(chessGame.getBoard()); | ||
return chessGame; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 나중에 room 을 구현하게 되면 여기에서는 room에 포함되는 boardId로만 대체해줘도 로딩에 문제가 없겠네요. |
||
|
||
playChess(chessGame, inputCommand); | ||
return new ChessGame(BoardFactory.createBoard(), true); | ||
} | ||
|
||
private void playChess(ChessGame chessGame, List<String> inputCommand) { | ||
while (isNotEnd(inputCommand)) { | ||
try { | ||
chessGame = createNewChessGame(chessGame, inputCommand); | ||
tryChessMove(chessGame, inputCommand); | ||
outputView.printBoard(chessGame.getBoard()); | ||
inputCommand = inputView.readGameCommand(); | ||
} catch (IllegalArgumentException exception) { | ||
System.out.println(exception.getMessage()); | ||
inputCommand = inputView.readGameCommand(); | ||
|
||
private void playChess(ChessGame chessGame) { | ||
Command command = inputView.readGameCommand(); | ||
|
||
while (!isGameEndCase(chessGame, command)) { | ||
chessGame = checkCreateNewGame(chessGame, command); | ||
|
||
checkMovePiece(chessGame, command); | ||
outputView.printBoard(chessGame.getBoard()); | ||
|
||
if (isGameDone(chessGame)) { | ||
break; | ||
} | ||
|
||
command = inputView.readGameCommand(); | ||
} | ||
} | ||
|
||
private boolean isNotEnd(final List<String> inputCommand) { | ||
return !inputCommand.get(COMMAND_INDEX).equals(END_COMMAND); | ||
private boolean isGameEndCase(final ChessGame chessGame, final Command command) { | ||
if (isGameEnd(chessGame, command)) { | ||
resultView.printGameEndWithSaving(); | ||
boardService.delete(BOARD_ID); | ||
boardService.save(BOARD_ID, chessGame.getBoard(), chessGame.isLowerTeamTurn()); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private boolean isGameEnd(final ChessGame chessGame, final Command command) { | ||
if (isCommandStatus(chessGame, command) || command.isGameStop()) { | ||
return true; | ||
} | ||
|
||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분은 별도의 로직 처리가 없다면 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 전 리뷰어분께서도 말씀해주셨는데, 습관이 돼서 자꾸 인지를 못하네요 ㅠㅠ 굳이 if문을 쓰지 않아도 이해하는데 문제가 없기 때문에 저도 카프카와 동의합니다! 꼼꼼하게 봐주셔서 감사합니다🙇🏻♂️ |
||
} | ||
|
||
private void tryChessMove(final ChessGame chessGame, final List<String> inputCommand) { | ||
if (inputCommand.get(COMMAND_INDEX).startsWith(MOVE_COMMAND)) { | ||
chessGame.move(inputCommand.get(SELECTED_PIECE), inputCommand.get(DESTINATION)); | ||
private boolean isCommandStatus(final ChessGame chessGame, final Command command) { | ||
if (command.isStatus()) { | ||
resultView.printScore(chessGame.calculateScoreOfUpperTeam(), chessGame.calculateScoreOfLowerTeam()); | ||
resultView.printWinner(chessGame.calculateScoreOfUpperTeam(), | ||
chessGame.calculateScoreOfLowerTeam()); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private ChessGame createNewChessGame(ChessGame chessGame, final List<String> inputCommand) { | ||
if (inputCommand.get(COMMAND_INDEX).equals(START_COMMAND)) { | ||
chessGame = new ChessGame(BoardFactory.createBoard()); | ||
private ChessGame checkCreateNewGame(final ChessGame chessGame, final Command command) { | ||
if (command.isCreateNewGame()) { | ||
chessGame.initGame(); | ||
} | ||
|
||
return chessGame; | ||
} | ||
|
||
private boolean isGameDone(final ChessGame chessGame) { | ||
if (isKingDead(chessGame)) { | ||
resultView.printGameEndWithDeleting(); | ||
boardService.delete(BOARD_ID); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private boolean isKingDead(final ChessGame chessGame) { | ||
if (chessGame.isKingDead() && chessGame.isUpperTeamWin()) { | ||
resultView.printWinnerIsUpperTeam(); | ||
return true; | ||
} | ||
|
||
if (chessGame.isKingDead() && !chessGame.isUpperTeamWin()) { | ||
resultView.printWinnerIsLowerTeam(); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private void checkMovePiece(final ChessGame chessGame, final Command command) { | ||
if (!command.isMove()) { | ||
return; | ||
} | ||
|
||
try { | ||
chessGame.move(command.findSelectedPiece(), command.findDestination()); | ||
} catch (IllegalArgumentException exception) { | ||
System.out.println(exception.getMessage()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package chess.dao.board; | ||
|
||
import java.util.Map; | ||
|
||
public interface BoardDao { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시 BoardDao와 BoardDaoImpl을 나눈 이유가 있을까요? |
||
|
||
void save(final int boardId, final Map<String, String> board, final boolean isLowerTeamTurn); | ||
|
||
Map<String, String> findById(final int boardId); | ||
|
||
boolean isLowerTeamTurnByBoardId(final int boardId); | ||
|
||
void remove(int boardId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
package chess.dao.board; | ||
|
||
import java.sql.Connection; | ||
import java.sql.DriverManager; | ||
import java.sql.PreparedStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
|
||
public class BoardDaoImpl implements BoardDao { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로컬에 DB를 올려서 테스트를 수행해 보았는데, 현재의 테이블 구조에 문제가 있어 변경이 필요해 보입니다.
이러한 구조는 적절하지 않습니다. 제가 생각하는 그 이유는 다음과 같습니다.
아직 database에 대해 학습하신 내용이 적어서, 충분히 할 수 있는 실수입니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 아래와 같은 개선이 필요하다고 생각해요.
물론 더 복잡한 구조로 구현할수도 있겠으나, 이 경우 오버엔지니어링이 될 위험이 있습니다. 해당 내용 수행 후, 테이블 생성시 사용된 DDL을 코멘트로 남겨주시면 참고해서 리뷰 진행하겠습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 문자열 1개로 압출하는 방법이 있었군요..! 왜 이 생각을 못했을까요 ㅠㅠ 저는 기존에는 64개(Position, Piece)를 모두 저장해야한다는 생각 때문에, 기본키를 누가 가져야하는지에 대해서 고민이 많았습니다. board_id가 가진다면 기본키는 중복이 안되기에 position, piece와 함께 저장이 모두 안되기 때문에 기본키를 없애거나, board_id와 position을 기본키로 설정하는 방식으로 진행했는데, 좋은 방법 하나 배워갑니다! 감사합니다 :) 테이블 생성 DDL은 다음과 같습니다. CREATE TABLE `chess`.`board` (
`board_id` INT NOT NULL,
`position` VARCHAR(300) NOT NULL,
`piece` VARCHAR(300) NOT NULL,
`isLowerTeamTurn` TINYINT NOT NULL,
PRIMARY KEY (`board_id`));
|
||
|
||
private static final String SERVER = "localhost:13306"; | ||
private static final String DATABASE = "chess"; | ||
private static final String OPTION = "?useSSL=false&serverTimezone=UTC"; | ||
private static final String USERNAME = "root"; | ||
private static final String PASSWORD = "root"; | ||
|
||
public Connection getConnection() { | ||
try { | ||
return DriverManager.getConnection("jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION, USERNAME, PASSWORD); | ||
} catch (final SQLException e) { | ||
System.err.println("DB 연결 오류:" + e.getMessage()); | ||
e.printStackTrace(); | ||
return null; | ||
} | ||
} | ||
|
||
@Override | ||
public void save(final int boardId, final Map<String, String> board, final boolean isLowerTeamTurn) { | ||
final String query = "INSERT INTO board (board_id, position, piece, isLowerTeamTurn) VALUES (?, ?, ?, ?)"; | ||
|
||
try (final PreparedStatement preparedStatement = getConnection().prepareStatement(query)) { | ||
|
||
for (final Entry<String, String> boardEntry : board.entrySet()) { | ||
final String position = boardEntry.getKey(); | ||
final String piece = boardEntry.getValue(); | ||
|
||
preparedStatement.setInt(1, boardId); | ||
preparedStatement.setString(2, position); | ||
preparedStatement.setString(3, piece); | ||
preparedStatement.setBoolean(4, isLowerTeamTurn); | ||
preparedStatement.addBatch(); | ||
} | ||
preparedStatement.executeBatch(); | ||
} catch (SQLException e) { | ||
e.printStackTrace(); | ||
throw new RuntimeException("board 저장 실패"); | ||
} | ||
} | ||
|
||
@Override | ||
public Map<String, String> findById(final int boardId) { | ||
final String query = "SELECT position, piece FROM board WHERE board_id = ?"; | ||
final Map<String, String> board = new HashMap<>(); | ||
try (final var connection = getConnection(); | ||
final var preparedStatement = connection.prepareStatement(query)) { | ||
|
||
preparedStatement.setString(1, String.valueOf(boardId)); | ||
ResultSet result = preparedStatement.executeQuery(); | ||
|
||
while (result.next()) { | ||
board.put(result.getString("position"), result.getString("piece")); | ||
} | ||
} catch (SQLException e) { | ||
e.printStackTrace(); | ||
} | ||
|
||
return board; | ||
} | ||
|
||
@Override | ||
public boolean isLowerTeamTurnByBoardId(final int boardId) { | ||
final String query = "SELECT isLowerTeamTurn FROM board WHERE board_id = ?"; | ||
|
||
try (final var connection = getConnection(); | ||
final var preparedStatement = connection.prepareStatement(query)) { | ||
|
||
preparedStatement.setString(1, String.valueOf(boardId)); | ||
|
||
final ResultSet result = preparedStatement.executeQuery(); | ||
|
||
while (result.next()) { | ||
return result.getBoolean("isLowerTeamTurn"); | ||
} | ||
} catch (SQLException e) { | ||
e.printStackTrace(); | ||
} | ||
return false; | ||
} | ||
|
||
@Override | ||
public void remove(final int boardId) { | ||
final String query = "DELETE FROM board WHERE board_id = ?"; | ||
|
||
try (final var connection = getConnection(); | ||
final var preparedStatement = connection.prepareStatement(query)) { | ||
preparedStatement.setString(1, String.valueOf(boardId)); | ||
preparedStatement.executeUpdate(); | ||
} catch (final SQLException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재는 BOARD_ID를 통해 단일 저장만 지원하도록 로직 작성해주셨고,
이후 추가 미션으로 나머지 테이블에 대해 구현을 수행하는 것으로 확인했습니다. 👍
다만 테이블의 구조 변경이 필요해 보이므로, 이를 수행한 뒤 이 부분도 변경해주시길 바랍니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@include42
카프카, 안녕하세요? 추가로 질문이 있습니다 !
개인적으로 DB로 데이터를 넣고, 받아오는 과정은 매우 중요하다고 생각합니다.
(만약 협업을 가정한다고 했을 때, 동료 개발자가 실수로 도메인 메서드를 DAO와 관련된 부분에서 실행하면 값이 바뀔 수도 있기 때문입니다!)
따라서
Controller <-> Service <-> DAO
에서 DTO를 적용해보고 싶은데,이런 경우 DTO에서
BoardUtil
에서 하는 역할을 불러서 수행을 시키고 보내는게 맞을까요?카프카의 생각은 어떤가요?? DTO의 책임은 어느정도까지인지 감이 잡히질 않아서 질문 남깁니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 제이!
말씀해주신 이유와 같은 이유로, 저도 DTO가 각 레이어간 통신에 필요하다고 생각합니다.
그리고 BoardUtil에서 하는 역할을 불러서 수행시킨다는 것은, Board를 가공하는 과정을 DTO가 책임져도 된다는 의미의 질문일까요?
저는 DTO의 책임은 값을 저장하고, 저장된 값을 전달한다 에 있다고 생각합니다. 그렇기 때문에 이상적인 DTO라면 생성자와 getter만 있어도 충분하겠지요.
다만 위 원칙대로라면 별도의 Converter 클래스가 Domain <-> DTO를 수행해야 하는데, 이러면서 클래스가 너무 많아져 관리가 어려울 경우 DTO에 정적 팩토리 메소드로 도메인을 인자로 받아 생성하는 정도는 허용하기도 합니다.
그 외의 연산 로직은 DTO에 필요할 것 같지는 않네요. 예를 들어 BoardUtil의 메소드들이 DTO에 있는 것은, 책임을 넘어서는 구조라고 생각이 됩니다. (정적 팩토리 메소드에서 호출해주는 정도는 가능하겠지만, 일단 DTO가 너무 복잡해지는 것은 좋지 않다고 생각합니다)
현재의 상황에서 제일 단순한 해결법은, 서비스 내에서 BoardUtil을 통해 필요한 값을 만들어주고 해당 값을 DTO에 넣어서 컨트롤러에 전달해주는 방법일 것 같습니다. 그러면 Controller는 해당 DTO를 다시 View에 넘기고요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@include42
그렇군요! 감사합니다 :)
말씀해주신 내용을 바탕으로 추가로 커밋하였습니다!