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단계 미션 제출합니다. #195

Merged
merged 22 commits into from
May 4, 2020

Conversation

giantim
Copy link

@giantim giantim commented May 4, 2020

작은곰 안녕하세요!
마지막까지 저와 함께해주셔서 감사합니다!

toneyparky added 22 commits May 4, 2020 16:58
schema.sql 에 game 테이블 정보 추가, history 테이블과 game 테이블 연결
application.properties test의 resources 디렉토리에 생성, h2 사용
Game entity 생성
schema.sql 테이블이 없는 경우에만 생성하도록 변경
GameRepositoryTest 테스트 코드 추가
GameRepository findAll() 오버라이드
GameRepositoryTest 조회 테스트 작성
Game Equals 오버라이드
GameRepository findByUuid() 추가
GameRepositoryTest 업데이트 테스트 작성
Game 어노테이션 컨벤션 수정
History 어노테이션 컨벤션 수정
History GameRef와 의존
schema.sql history_game테이블 생성
HistoryRepositoryTest 테스트 코드 추가
Game histories 추가
schema.sql history_game 테이블 삭제
GameRepositoryTest 게임에 해당하는 전체 히스토리 조회 테스트 추가
SpringChessController /new로 post 요청을 보내는 라우트 추가
SpringDataJDBCChessService createGameBy로 메서드명 수정
SpringChessController id를 라우트에 추가
SpringDataJDBCChessService id를 이용해서 해당 게임 정보를 가져오도록 수정
chess.js id를 이용하여 리퀘스트를 보내도록 수정
SpringChessController result get 라우트 구현
SpringDataJDBCChessService updateCanContinueToFalse를 사용
chess.js 종료시 result.hbs로 이동
SpringChessController /games get 라우트 추가
GamesDto 추가
GameRepository findAvailableGames로 메서드명 변경
SpringDataJDBCChessService selectAvailableGames 메서드 추가
index.js 게임 목록을 가져와 보여주는 기능 추가
chess.hbs 나가기 링크 추가
SpringDataJDBCChessService 리팩토링에 따른 수정
HistoryRepository.java 삭제
SpringChessController 예외 정보를 담은 Dto 생성해 반환
SpringDataJDBCChessService 예외 발생 시 예외 전달
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.

연휴기간동안 마무리하느라 고생하셨습니다!
항상 코드를 보면서 많은 노력을 했다고 생각이 들었는데, 좋은 결과가 나왔기를 바랍니다. :)
몇가지 피드백을 남겼는데, 문제가 된다는 것보다는 생각을 해보면 좋겠다는 의견들이에요.
한번 확인해주세요~

@Autowired
private SpringDataJDBCChessService springChessService;
@Autowired
private SpringDataJDBCChessService springDataJDBCChessService;

Choose a reason for hiding this comment

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

https://madplay.github.io/post/why-constructor-injection-is-better-than-field-injection

의존성 주입에 대한 방법에 대해 생각해보면 좋을 것 같아요 :)

private Boolean canContinue;
private Set<History> histories = new HashSet<>();

public Game() {

Choose a reason for hiding this comment

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

사용하지 않는다면 private로 접근제어자를 변경하여 닫아주면 어떨까요?


@Repository
public interface GameRepository extends CrudRepository<Game, Long> {
@Query("SELECT * FROM game WHERE can_continue = true")

Choose a reason for hiding this comment

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

👍👍

@Id
private Long id;
private String start;
private String end;

public History() {

Choose a reason for hiding this comment

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

Game의 생성자와 동일하게 접근제어자를 바꾸면 좋을 것 같아요 :)

import java.util.stream.Collectors;

@Service
public class SpringDataJDBCChessService {
@Autowired
private HistoryRepository historyRepository;
private GameRepository gameRepository;

Choose a reason for hiding this comment

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

이 또한 위와 같이 의존 주입에 대해 생각해보아요

.map(history -> new MovingPosition(history.getStart(), history.getEnd()))
private List<MovingPosition> selectAllHistoryBy(Long id) {
Game game = gameRepository.findById(id).orElseThrow(() ->
new IllegalArgumentException("없는 게임 id 입니다."));

Choose a reason for hiding this comment

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

👍

ChessGame chessGame = new ChessGame();
load(chessGame);
load(chessGame, id);

Choose a reason for hiding this comment

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

load 메서드의 선언부를 move 메서드 아래로 옮기면 좋을 것 같네요.

@gracefulBrown gracefulBrown merged commit 64a5f56 into woowacourse:giantim May 4, 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