-
Notifications
You must be signed in to change notification settings - Fork 122
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
[2단계 - 웹 자동차 경주] 제이미(임정수) 미션 제출합니다. #183
Changes from 16 commits
5d4c6c0
dc95ea0
eec0887
a8e5efe
ea1c77e
05e9802
b457b72
372aa11
ff4645b
4e460d0
ddd3a5a
3fbcf65
4994fdc
1475dcc
f998022
604966f
ab3dd50
ce5e165
00f45a5
72a9705
82f8a15
a170c7b
be898cb
58dfcf1
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 |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package racingcar; | ||
|
||
import racingcar.controller.RacingCarConsoleController; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
RacingCarConsoleController racingCarConsoleController = new RacingCarConsoleController(); | ||
racingCarConsoleController.runGame(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package racingcar.controller; | ||
|
||
import racingcar.domain.Cars; | ||
import racingcar.service.RacingCarService; | ||
import racingcar.util.RandomNumberGenerator; | ||
import racingcar.view.InputView; | ||
import racingcar.view.OutputView; | ||
|
||
public class RacingCarConsoleController { | ||
private final InputView inputView = new InputView(); | ||
private final OutputView outputView = new OutputView(); | ||
private final RacingCarService racingCarService = new RacingCarService(); | ||
private final RandomNumberGenerator randomNumberGenerator = new RandomNumberGenerator(); | ||
|
||
public void runGame() { | ||
Cars cars = initCarData(); | ||
movePerRounds(cars, setTryCount()); | ||
outputView.printWinner(cars); | ||
} | ||
|
||
private Cars initCarData() { | ||
outputView.printCarNameMessage(); | ||
|
||
try { | ||
String carNames = inputView.inputCarNames(); | ||
return racingCarService.getCars(carNames); | ||
} catch (Exception e) { | ||
System.out.println(e.getMessage()); | ||
return initCarData(); | ||
} | ||
} | ||
|
||
private int setTryCount() { | ||
outputView.printTryCountMessage(); | ||
|
||
try { | ||
int tryCount = inputView.inputTryCount(); | ||
return racingCarService.getTrialCount(tryCount); | ||
} catch (Exception e) { | ||
System.out.println(e.getMessage()); | ||
return setTryCount(); | ||
} | ||
} | ||
|
||
private void movePerRounds(Cars cars, int tryCount) { | ||
outputView.printResultMessage(); | ||
racingCarService.playGame(cars, tryCount, randomNumberGenerator); | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package racingcar.controller; | ||
|
||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestBody; | ||
import org.springframework.web.bind.annotation.RestController; | ||
import racingcar.dto.GameInforamtionDto; | ||
import racingcar.dto.GameResultDto; | ||
import racingcar.service.RacingCarService; | ||
import racingcar.util.NumberGenerator; | ||
|
||
import java.util.List; | ||
|
||
@RestController | ||
public class RacingCarWebController { | ||
|
||
private final RacingCarService racingCarService; | ||
private final NumberGenerator numberGenerator; | ||
|
||
public RacingCarWebController(RacingCarService racingCarService, NumberGenerator numberGenerator) { | ||
this.racingCarService = racingCarService; | ||
this.numberGenerator = numberGenerator; | ||
} | ||
|
||
@PostMapping("plays") | ||
public GameResultDto createGame(@RequestBody GameInforamtionDto gameInforamtionDto) { | ||
return racingCarService.play(gameInforamtionDto, numberGenerator); | ||
} | ||
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. ResponseEntity가 아닌 객체를 바로 반환하도록 함 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. Controller와 Service의 책임을 어떻게 부여해야 할지 감이 오지 않습니다. 레벨1에서는 콘솔을 view로 사용하였고 Service 레이어의 필요성을 느끼지 못했기에 현재 Service에 있는 로직은 Controller에 있었습니다. 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. 자세한 답변 감사합니다! |
||
|
||
@GetMapping("plays") | ||
public List<GameResultDto> loadAllGame() { | ||
return racingCarService.findAllGame(); | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package racingcar.domain; | ||
package racingcar.dto; | ||
|
||
public class GameInforamtionDto { | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package racingcar.domain; | ||
package racingcar.dto; | ||
|
||
import java.util.List; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package racingcar.domain; | ||
package racingcar.dto; | ||
|
||
public class RacingCarDto { | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package racingcar.repository; | ||
|
||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.stereotype.Repository; | ||
import racingcar.domain.Car; | ||
import racingcar.dto.RacingCarDto; | ||
|
||
import java.util.List; | ||
|
||
@Repository | ||
public class RacingCarDao { | ||
|
||
private JdbcTemplate jdbcTemplate; | ||
|
||
public RacingCarDao(JdbcTemplate jdbcTemplate) { | ||
this.jdbcTemplate = jdbcTemplate; | ||
} | ||
|
||
public void insert(Car car, long resultId) { | ||
String sql = "insert into racing_cars (name, position, result_id) values (?, ?, ?)"; | ||
jdbcTemplate.update(sql, car.getName(), car.getLocation(), resultId); | ||
} | ||
|
||
public List<RacingCarDto> findBy(long resultId) { | ||
String sql = "select name, position from racing_cars where result_id = ?"; | ||
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. 현재 제 쿼리문과 Dto를 살펴보면 table 데이터의 한 행을 가져오는 것이 아니라 행에서 필요한 정보만 가져오고 있습니다. 이를 고민하며 추가적으로 든 의문은 DTO와 Entity의 차이입니다. 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. 자세한 답변 감사합니다! 처음에는 사용하지 않는 데이터도 있으니, dto를 사용해야 하지 않을까? 라고, 생각했습니다. 추가적으로 만약 전체가 아니라 특정 데이터들만 가져오게 된다면 해당 객체는 무슨 객체에 속하는 것일까요? 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. 또한, id를 null로 설정하고 보내라는 이야기가 있는 이 부분에 대해 제대로 이해하지 못했습니다. 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. 적어주신대로 select * 를 지양하는 것은 좋은 습관이라고 생각합니다.
다만 db의 값 중 필요한 값 일부만을 가져오는 것이 현재 미션에서 필요할지에 대해서도 의문이 들기는 하지만요. 그리고 특정 데이터들만 가져오게 된다면, 아무래도 entity보다는 ResultSet을 VO 클래스에 매핑하게 되지 않을까 생각이 됩니다. 저번 리뷰에서 제가 그러한 경우에 사용하는 클래스 역시 엔티티라고 지칭했는데,그보다는 VO라는 표현이 더 맞지 않을까 싶었어요. |
||
return jdbcTemplate.query(sql, | ||
(rs, rowNum) -> { | ||
String name = rs.getString("name"); | ||
int position = rs.getInt("position"); | ||
RacingCarDto racingCarDto = new RacingCarDto(name, position); | ||
return racingCarDto; | ||
}, resultId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
package racingcar.dao; | ||
package racingcar.repository; | ||
|
||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.jdbc.support.GeneratedKeyHolder; | ||
import org.springframework.jdbc.support.KeyHolder; | ||
import org.springframework.stereotype.Repository; | ||
|
||
import java.sql.PreparedStatement; | ||
import java.util.List; | ||
|
||
@Repository | ||
public class ResultDao { | ||
|
@@ -28,4 +29,14 @@ public long insert(int trialCount, String winners) { | |
|
||
return keyHolder.getKey().longValue(); | ||
} | ||
|
||
public List<Long> findAllId() { | ||
String sql = "select id from results"; | ||
return jdbcTemplate.queryForList(sql, Long.class); | ||
} | ||
|
||
public String findWinnerBy(long id) { | ||
String sql = "select winners from results where id = ?"; | ||
return jdbcTemplate.queryForObject(sql, String.class, id); | ||
} | ||
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. 현재 id를 모두 가져와 results 테이블에서는 각 아이디의 우승자를 가져오고 racing_cars 테이블에서는 각 아이디를 result_id로 갖고 있는 행들에 대한 List를 가져오게 되어 있습니다. 그런데 쿼리문 중 join이 있는 것을 알고 있는데 이를 사용하는 것이 더 효율적인지 궁금합니다. 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. 만약 dao에 join을 꼭 적용해야 한다면, 저라면 join으로 얻게 되는 값과 매핑되는 entity 클래스를 만들어주고 그 엔티티에 대한 연산만 수행하도록 dao의 역할을 제한할 듯 하네요. 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. join 사용에 대한 의견 감사합니다! join과 관련된 추가적인 질문으로 현재 foreign key를 사용하고 있습니다. 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. join으로 얻게 되는 값도 위의 리뷰에 기재한 것처럼, Entity라기보다는 ResultSet을 VO 클래스에 매핑하는 식이 되지 않을까 싶네요.
그리고 FK를 사용하는 것은 join과는 별개로 좋다고 저는 생각해요! |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,85 @@ | ||
package racingcar.service; | ||
|
||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Service; | ||
import racingcar.dao.RacingCarDao; | ||
import racingcar.dao.ResultDao; | ||
import racingcar.domain.Car; | ||
import racingcar.domain.Cars; | ||
import racingcar.dto.GameInforamtionDto; | ||
import racingcar.dto.GameResultDto; | ||
import racingcar.dto.RacingCarDto; | ||
import racingcar.repository.RacingCarDao; | ||
import racingcar.repository.ResultDao; | ||
import racingcar.util.NumberGenerator; | ||
import racingcar.validation.Validation; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
@Service | ||
public class RacingCarService { | ||
|
||
private final ResultDao resultDao; | ||
private final RacingCarDao racingCarDao; | ||
private ResultDao resultDao; | ||
private RacingCarDao racingCarDao; | ||
|
||
@Autowired | ||
public RacingCarService(ResultDao resultDao, RacingCarDao racingCarDao) { | ||
this.resultDao = resultDao; | ||
this.racingCarDao = racingCarDao; | ||
} | ||
|
||
public void insertGame(int trialCount, Cars cars) { | ||
public RacingCarService(){} | ||
|
||
public GameResultDto play(GameInforamtionDto gameInforamtionDto, NumberGenerator numberGenerator) { | ||
Cars cars = getCars(gameInforamtionDto.getNames()); | ||
int trialCount = getTrialCount(gameInforamtionDto.getCount()); | ||
|
||
List<RacingCarDto> racingCars = playGame(cars, trialCount, numberGenerator); | ||
insertGame(trialCount, cars); | ||
|
||
return new GameResultDto(cars.getWinnerCars(), racingCars); | ||
} | ||
|
||
public Cars getCars(String names) { | ||
Validation.validateCarNames(names); | ||
return new Cars(names); | ||
} | ||
|
||
public int getTrialCount(int trialCount) { | ||
Validation.validateTryCount(trialCount); | ||
return trialCount; | ||
} | ||
|
||
public List<RacingCarDto> playGame(Cars cars, int trialCount, NumberGenerator numberGenerator) { | ||
for (int count = 0; count < trialCount; count++) { | ||
cars.moveForRound(numberGenerator); | ||
} | ||
|
||
List<RacingCarDto> racingCars = new ArrayList<>(); | ||
for (Car car : cars.getCars()) { | ||
RacingCarDto racingCarDto = new RacingCarDto(car.getName(), car.getLocation()); | ||
racingCars.add(racingCarDto); | ||
} | ||
return racingCars; | ||
} | ||
|
||
private void insertGame(int trialCount, Cars cars) { | ||
long resultId = resultDao.insert(trialCount, cars.getWinnerCars()); | ||
|
||
for (Car car : cars.getCars()) { | ||
racingCarDao.insert(car, resultId); | ||
} | ||
} | ||
|
||
public List<GameResultDto> findAllGame() { | ||
List<Long> gameIds = resultDao.findAllId(); | ||
List<GameResultDto> gameResults = new ArrayList<>(); | ||
|
||
for (Long gameId : gameIds) { | ||
String winners = resultDao.findWinnerBy(gameId); | ||
List<RacingCarDto> racingCars = racingCarDao.findBy(gameId); | ||
gameResults.add(new GameResultDto(winners, racingCars)); | ||
} | ||
|
||
return gameResults; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package racingcar.view; | ||
|
||
import racingcar.constant.ExceptionMessage; | ||
|
||
import java.util.Scanner; | ||
|
||
public class InputView { | ||
private final Scanner scanner = new Scanner(System.in); | ||
|
||
public String inputCarNames() { | ||
return scanner.nextLine(); | ||
} | ||
|
||
public int inputTryCount() { | ||
try { | ||
return scanner.nextInt(); | ||
} catch (Exception e) { | ||
throw new IllegalArgumentException( | ||
ExceptionMessage.TRY_COUNT_NOT_NUMBER_MESSAGE.getExceptionMessage()); | ||
} | ||
} | ||
} |
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.
콘솔과 웹 게임의 중복 코드를 제거해 주기 위해 ConsoleController에서 사용하는 메서드들을 public으로 바꿔주었습니다.
이와 같은 방법이 맞는 것인지 아직 감이 오지 않는 상태입니다.
카프카의 의견이 궁금합니다...!
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.
사실 보통의 상황이라면 이렇게 콘솔과 웹을 동시에 구현하지는 않을듯해요. 그러나 미션 요구사항이므로...특수한 상황임을 감안하면 이해가 되는 부분입니다.
일반적인 구조라면, Controller는 요청에 대한 라우팅만 수행하는 것이 맞기는 합니다. 위의 runGame 메소드같은 경우도, 요청이 들어올때 Service에서 수행해줘야 하지요. 일단 RacingCarWebController에서 사양에 맞게 잘 구현되어 있으므로, 이 부분에 대해서는 감안하겠습니다.
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.
RacingCarConsoleController 클래스의 runGame()에 존재하는 로직은 모두 View를 위한 로직인데 그래도 Service에 있어야 하는 것일까요?
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.
View를 위한 로직이라는 말이 참 애매한데... 현재 보면 컨트롤러가 View 레이어와 강한 의존을 가지고 있다고 느껴져요.
만약 완벽하게 리팩토링을 하고 싶다면, outputview와 inputview를 json DTO를 통해 통신하도록 완벽히 구현해서 WebController와 동일한 서비스를 사용하도록 해볼 수 있어 보이긴 합니다. 다만 이미 WebController 부분의 구현에서 해당 스펙을 구현했으니, 이번 미션에서 그 정도까지의 작업을 할 필요는 없다고 판단했습니다.
MVC 구조에서 서비스가 하는 역할에 대해 잘 이해하고 있다면, 크게 문제가 되어 보이지는 않습니다.