[1단계 - 웹 자동차 경주] 밀리(김미성) 미션 제출합니다. #94
Conversation
궁금한 점
|
There was a problem hiding this comment.
안녕하세요 밀리.
전체적으로 잘 작성해주셔서 가벼운 피드백만 남겼습니다 ㅎㅎ. 확인 부탁드려요.
그리고 질문주신 부분들을 살펴보자면,
첫번째로 컨트롤러가 서비스의 여러 메서드를 호출해도 되는지 질문주셨는데, 개인적으로는 하나만 호출하는게 좋다고 생각합니다.
이는 서비스 메서드 단위를 고민해봐야하는데요, 서비스는 도메인 행위 단위보다 큰 범위라고 생각해서 사용자의 특정 행위를 단위로 메서드가 작성하는 편입니다. 레이싱 게임의 경우 "레이싱 게임 수행" 자체가 하나의 서비스 메서드 단위이며, 요청을 통한 게임 생성, 게임 진행, 결과 출력 까지 하나의 메서드로 진행된다고 생각합니다.
만약 이번 레이싱게임 미션에서 사용자가 게임 생성, 게임진행, 결과 출력을 3단계로 걸쳐서 할수 있었다면, Controller의 path도 3단계로 쪼개지고, 그에 따라 Service도 3개의 메서드가 생기도록 설계할거 같습니다.
물론 이건 저만의 기준이고, 밀리가 고민해보시고 어떤게 가독성과 유지보수 측면에서 좋은지 고민해보시면 좋을것 같아요.
두번째로 계층간 DTO사용은, 실제 현업에서도 DTO가 굉장히 많아지는게 맞습니다. 우선 DTO를 만드는 이유는 외부 요구사항으로부터 도메인의 변경을 막기 위함인데요. View의 요구사항으로부터 도메인 변경을 막기위해 Controller단에는 필수적으로 Dto를 사용하는것 같습니다.
특히, Controller에서 사용하는 DTO들은 모든 Controller메서드마다 만들어지는데요, 만약 공용으로 사용할 경우 특정 API 의 스펙 변경이 다른 API에 영향을 미칠 수 있기 때문에, 동일한 요청/응답이더라도 될수 있으면 API 마다 DTO를 만드는 편이 좋습니다. 다만, DAO와 통신할때는 일반적으로 테이블과 도메인의 형태가 거의 비슷하고, 변경가능성이 적기 때문에 도메인 객체를 직접 활용하는 편입니다.
세번째로 Dao 테스트를 위해서 find 메서드를 추가하는것은 현재 미션의 특징 때문인것 같습니다.
데이터들이 저장되는 이유는 조회하기 위해서입니다. 따라서 일반적인 애플리케이션이라면 테스트 작성하기 전에 find 메서드가 있을거에요. 그러니 걱정안하셔도 됩니다.
네번째로 Validation같은 경우는, Controller레이어나, Service 레이어 중에서 적절하다고 생각되시는 부분에서 검증을 해주시면 됩니다. 어느쪽이든 저는 괜찮다고 생각합니다.
코멘트를 적다보니 너무 과한 내용을 설명드린게 아닌가 싶네요. 적절히 그런게 있구나~정도로 알아주시면 될것같습니다.
피드백은 가벼운 내용이니 2단계 진행하실때 반영부탁드려요 :)
수고 많으셨습니다~
| import racingcar.service.RacingCarService; | ||
|
|
||
| @RestController | ||
| public class RacingCarController { |
There was a problem hiding this comment.
이름이... ㅋㅋㅋ Web전용 Controller라는 네이밍으로 바꿔주시는게 어떨까요?
| public class GameRequest { | ||
|
|
||
| @NotBlank(message = "하나 이상의 이름을 입력해야 합니다.") | ||
| private final String names; |
There was a problem hiding this comment.
프론트에서 요청을 보낼 때 Requestbody의 값을 List로 받는것을 말씀하시는건가요??
그렇다면 프론트 단에서 names를 넘겨줄 때 List 형식으로 보내줘야 할 것 같은데, 그렇다면 js 코드를 수정하는 방법 밖에는 생각이 나지 않습니다..! 혹시 다른 방법이 있는걸까요?
There was a problem hiding this comment.
아.. js코드는 템플릿이었나보네요.. ㅎㅎ
저때는 js코드도 직접 작성하는부분이 있어서 몰랐습니다. 이 피드백은 넘어가셔도됩니다!
|
|
||
| private static final int IS_WINNER = 1; | ||
| private final JdbcTemplate jdbcTemplate; | ||
| private final SimpleJdbcInsert insertActor; |
There was a problem hiding this comment.
jdbcTemplate을 사용하지 않고 insertActor를 사용하시는 이유가 있을까요?
There was a problem hiding this comment.
jdbcTemplate으로 insert를 하고 생성된 id 값을 받아오기 위해서는 keyholder를 사용해야하는데,
그 과정의 코드가 가독성이 좋지 않다고 생각하여 insertActor를 사용했습니다!
학습테스트를 진행하면서 jdbcTemplate의 update 메서드와 SimpleJdbcInsert의 insertActor가 역할은 같은데 왜 구분이 되어있을까 고민했었습니다. 결론은 jdbcTemplate의 update는 ?에 대응하는 값을 인자로 넣어주게 되는데, 이때 인자가 많을수록 실수할 확률이 높아진다고 생각했습니다. 반면 insertActor는 Map으로 필드값을 명시해주면서 put하게 되어 상대적으로 안전하다고 생각했습니다.
보통 insert 구문을 사용할 때 jdbcTemplate을 사용하나요??
There was a problem hiding this comment.
어떤걸 사용하셔도 괜찮습니다. 다만 JdbcTemplate로 insert를 할수 있도록 추상화 해놓았는데 어떤 이유로 쓰셨는지 궁금해서 여쭤밨습니다.
| public class GlobalExceptionHandler { | ||
|
|
||
| @ExceptionHandler(MethodArgumentNotValidException.class) | ||
| @ResponseStatus(HttpStatus.NOT_ACCEPTABLE) |
There was a problem hiding this comment.
찾아보니 클라이언트의 잘못된 요청값은 400 status 를 보내는게 맞는 것 같습니다. 수정했습니다!
| @Service | ||
| public class RacingCarService { | ||
|
|
||
| private static final String DELIMITER = ","; |
There was a problem hiding this comment.
입력을 split하는 책임을 racingGameService 가 알아야할까요?
There was a problem hiding this comment.
기존 콘솔 어플리케이션에서는 inputView에서 split을 하도록 구현했었습니다.
따라서 입력값에 대한 형태 변환은 서비스보다는 컨트롤러가 적합한 것 같습니다!
혹은 Car를 생성하는 CarFactory에서 split을 해도 좋을 것 같습니다. 하지만 기존 CarFactory를 수정하면 변경되는 기존 코드가 많아서 컨트롤러에서 하도록 수정했습니다!
| jdbcTemplate.execute(DROP_TABLE_CAR); | ||
| jdbcTemplate.execute(DROP_TABLE_GAME); | ||
| jdbcTemplate.execute(CREATE_TABLE_GAME); | ||
| jdbcTemplate.execute(CREATE_TABLE_CAR); | ||
|
|
There was a problem hiding this comment.
@Sql 을 통해서도 하실수 있습니다 ㅎㅎ
사용법을 한번 공부해보시면 편하실거에요
| import racingcar.strategy.MovingStrategy; | ||
|
|
||
| @Service | ||
| public class RacingCarService { |
| int gameId = racingCarService.play(gameRequest); | ||
| List<CarDto> winners = racingCarService.findWinners(gameId); | ||
| String winnerNames = winners.stream() | ||
| .map(CarDto::getName) | ||
| .collect(Collectors.joining(DELIMITER)); | ||
| List<CarDto> cars = racingCarService.findCars(gameId); | ||
|
|
There was a problem hiding this comment.
service 메서드를 하나로 두는건 어떨까요?
service의 메서드는 하나의 비즈니스 수행 단위라고 봐주시면 좋을것 같습니다 :)
There was a problem hiding this comment.
질문했던 부분인데, 답변 정말 감사합니다!
현재는 여러 행동을 하나의 요청으로 처리하기 때문에 메서드 하나로 두게 되면 너무 많은 일을 한다고 생각해서 나눴던 것 같습니다!
저도 하나의 요청에 하나의 메서드만을 호출하는 것이 좋다고 생각합니다.
반영하여 수정하겠습니다!
| @@ -0,0 +1,9 @@ | |||
| package racingcar.strategy; | |||
|
|
|||
| public class FixedMovingStrategy implements MovingStrategy { | |||
| } | ||
|
|
||
| List<Car> cars = racingGame.getCars(); | ||
| convertDto(cars).forEach(car -> carDao.updatePosition(car, racingGame.getId())); |
There was a problem hiding this comment.
Service - Dao 통신과 Controller - Service 통신 시 같은 객체를 공유하는건 위험할 수 있습니다. View의 요구사항에 변경에 따라 Dao로직에도 문제가 생길수도 있거든요.
그리고 개인적으로는 Dao 와 통신할때 굳이 Dto를 사용해야하나 싶기도 하네요 ㅎㅎ
There was a problem hiding this comment.
Service - Dao 통신과 Controller - Service 통신 시 같은 객체를 공유 하는 부분을 찾을 수 없어서 질문드립니다..!
컨트롤러에서 GameRequest를 서비스로 직접 전달한 후, 그 값들을 필요에 맞게 꺼내서 사용하였고, 직접 GameRequest를 Dao에 넘기는 부분이 없습니다. 디디가 말씀하신 공유하는 부분이 GameRequest가 아니라면 혹시 어떤 객체인가요??
혹시 아래 findWinners 메서드처럼 Dao에서 받아온 값을 그대로 컨트롤러로 전달하는 부분을 말씀하시는건가요?
public List<CarDto> findWinners(final int gameId) {
return carDao.findWinners(gameId);
}There was a problem hiding this comment.
추가적으로, 그렇다면 디디는 Dao와 통신할 때 그냥 객체를 직접 넘기는 방식으로 구현하시나요?
Dto를 사용하는 이유는 계층 간 데이터 전달을 하기 위함이고, 계층간 데이터가 오고가면서 변질될 우려가 있기 때문에 사용해야 된다고 알고 있습니다!
혹시 Dao와 통신할 때 Dto를 굳이 사용하지 않아도 되는 이유를 알려주실 수 있나요?🤔
There was a problem hiding this comment.
코드 관리 측면에서 DTO를 생략할 수 있다고 생각했습니다.
이부분은 나중에 Repository 개념을 통해서 학습하실거라서, 아직은 고민 안해주셔도 될것 같아요!
안녕하세요 디디! 5기 밀리입니다 😊
리뷰 잘 부탁드립니다!
스프링을 사용해보니 공부해야할 부분이 정말 많은 것 같습니다 🥲
질문은 따로 아래에 남겨놓겠습니다!
감사합니다!