[2단계 - 웹 자동차 경주] 허브(방대의) 미션 제출합니다. #128
Conversation
| carDao.saveAll(gameId, cars); | ||
| final GameEntity gameEntity = racingGameMapper.toGameEntity(gameRequest.getCount()); | ||
| final int gameId = gameDao.saveAndGetId(gameEntity) | ||
| .orElseThrow(() -> new IllegalArgumentException("게임 저장에 실패하였습니다.")); |
There was a problem hiding this comment.
던지는 예외에서 조금 고민이 있네요!
엔티티를 정상적으로 생성한 이후에 데이터베이스에 저장하는 과정에서 발생한 예외에 대한 처리로 보여지는데요. IllegalArgumentException이 아닌 다른 예외를 던져보는것은 어떨까요?
There was a problem hiding this comment.
말씀해주신대로 다른 예외를 던지는 것이 좋아보입니다.
현재는 저장이 되지 않은 것이니 IllegalStateException을 적용해보도록 하겠습니다! 👍
| public class CarEntity { | ||
| private Integer id; | ||
| private String name; | ||
| private int position; | ||
| private boolean winner; | ||
| private Integer gameId; |
There was a problem hiding this comment.
앗 이 부분 final을 빼먹었군요 😢 꼼꼼하게 봐주셔서 감사합니다!
| () -> assertThat(carEntity.getId()).isPositive(), | ||
| () -> assertThat(carEntity.getName()).isEqualTo("car1"), | ||
| () -> assertThat(carEntity.getPosition()).isEqualTo(1), | ||
| () -> assertThat(carEntity.isWinner()).isEqualTo(false), |
There was a problem hiding this comment.
boolean을 검증할때는 isFalse() / isTrue() 등을 활용해보면 어떨까요? 😉
There was a problem hiding this comment.
좋은 것 같습니다! 바로 적용하겠습니다 😄
|
|
||
| // then | ||
| assertThat(id).isPositive(); | ||
| assertThat(gameId.isPresent()).isTrue(); |
There was a problem hiding this comment.
Optional의 존재를 검사하는 isPresent()를 활용해보면 어떨까요?😉
| assertThat(gameId.isPresent()).isTrue(); | |
| assertThat(gameId).isPresent(); |
There was a problem hiding this comment.
Optional 검사할 때 isPresent를 사용할 수 있군요! 처음 알았습니다. 👍👍👍
|
안녕하세요 라빈! 아침에 리뷰해주셔서 편하게 리뷰 반영할 수 있어서 너무 좋았습니다. 😄 |
|
|
||
| final List<CarEntity> carEntities = racingGameMapper.toCarEntities(game, gameId); | ||
| carDao.saveAll(carEntities); | ||
|
|
||
| return GamePlayResponseDto.of(game.findWinners(), game.getCars()); | ||
| } | ||
|
|
||
| @Transactional | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
와,,,, 허브 굉장하시네요 👍
지금과 같이 하나의 클러스터를 사용하는 경우에는 영향이 없겠지만 나중에 규모있는 애플리케이션을 작성할 때는 항상 신경써야 하는 부분이에요. 정말 선행학습이 잘 되어있으시네요 👏
There was a problem hiding this comment.
칭찬을 받으니 너무 좋습니다👍🏻 라빈의 기운 받고 꾸준히 정진 하겠습니다!!
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| @Transactional | ||
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| @SuppressWarnings("NonAsciiCharacters") | ||
| public class RacingGameIntegrationTest { |
There was a problem hiding this comment.
요거는 저희 팀에서 관리하는 컨벤션인데요.
하나의 클래스에 다양한 어노테이션을 붙일 때 중요도가 높은 어노테이션일 수록 클래스 이름에 가깝게 명시해주고 있어요. 해당 클래스가 어떤 도움을 받고 있는지 빠르게 파악하기 위해서요.
중요도를 선정하는 기준은 스프링 프레임워크를 사용하고 있고, 해당 클래스가 빈인지, 설정 파일인지 등을 명시해주는 어노테이션을 가장 먼저 사용하고 그 이후에 부가적인 어노테이션들을 붙이고 있어요. 이 테스트 클래스를 기준으로 본다면 @SpringBootTest -> @AutoConfigureMockMvc -> @transactional -> @DisplayNameGeneration -> @SuppressWarnings 순이 되겠네요!
이 피드백은 반드시 따르거나 지켜야하는건 아니고 허브가 어떤 방식이 클래스를 봤을 때 무엇을 하는지 파악하기 쉬운지 고민해보셔도 좋을거 같아요 😉
There was a problem hiding this comment.
중요도에 의해 순서를 결정하는 부분 좋은 것 같습니다. 확실히 해당 클래스가 어떤 애너테이션에 제일 영향을 많이 받는지 한 눈에 이해할 수 있을 것 같아요!
다음 미션부터 적용하도록 해보겠습니다!
실무에서 직접 사용하고 있는 컨벤션에 대한 부분을 리뷰로 남겨주셔서 정말 좋은 것 같습니다!👍🏻👍🏻👍🏻
안녕하세요 라빈🍦 잘 지내셨나요?
2단계 구현해서 제출합니다!
요구사항 이외에 추가 및 수정된 부분은 다음과 같습니다.
그 외 작게 작게 수정했습니다!