5단계 - 자동차 경주(리팩터링) #52
Conversation
build.gradle.kts
Outdated
| @@ -1,5 +1,6 @@ | |||
| plugins { | |||
| kotlin("jvm") version "1.8.10" | |||
| id("org.jlleitschuh.gradle.ktlint") version "10.3.0" | |||
There was a problem hiding this comment.
앗 몰래 다시 쓰고 있었는데 들켰네요. 제거하도록 하겠습니다 🫡
| fun main() { | ||
| val app = RacingGameController( | ||
| userInterface = ConsoleUserInterface(), | ||
| racingGameView = StringRacingGameView(), | ||
| racingGameService = RacingGameService( | ||
| carFactory = CarFactory(), | ||
| racingGameRepository = InMemoryRacingGameRepository(), | ||
| ), | ||
| ) | ||
|
|
||
| val gameId: GameId = app.initialize() | ||
| app.playing(gameId = gameId) | ||
| app.showResult(gameId = gameId) | ||
| } |
There was a problem hiding this comment.
앗 정줄 놓고 코딩해서 똑같은걸 2번 작성했네요. 여기 있는건 삭제하도록 할게요! 😀
|
|
||
| override fun equals(other: Any?): Boolean { | ||
| if (this === other) return true | ||
| if (javaClass != other?.javaClass) return false | ||
|
|
||
| other as RacingGame | ||
|
|
||
| if (id != other.id) return false | ||
| if (carList != other.carList) return false | ||
| if (tryCount != other.tryCount) return false | ||
| return gameState == other.gameState | ||
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| var result = id.hashCode() | ||
| result = 31 * result + carList.hashCode() | ||
| result = 31 * result + tryCount | ||
| result = 31 * result + gameState.hashCode() | ||
| return result | ||
| } |
There was a problem hiding this comment.
모든 프로퍼티에 대한 equals hashcode 구현이 되었는데요, 요러면 id 가 일치하더라도 나머지 값이 다르면 서로 다른 객체로 인지하게 될거 같아요 😱
There was a problem hiding this comment.
id 를 만들어놓고 equals를 무지성으로 작성했네요..😵 둘 다 id만 비교하도록 바꿔놓겠습니다!
| fun createGame(): GameId { | ||
| val game: RacingGame = racingGameRepository.create() | ||
|
|
||
| return game.id | ||
| } |
There was a problem hiding this comment.
repository 는 각 원소들을 마치 컬렉션처럼 다룰 수 있는 개념이라고 생각해요.
때문에 우선 RacingGame 객체를 생성하는 것은 repository 의 관할 밖이고,
저장하여 ID 를 발급 받는 것 까지가 repository 의 역할이라고 생각하는데
(마치 key-value 구조에 저장해서 key 를 기억하는 느낌)
지원님은 어떻게 생각하시나요?
There was a problem hiding this comment.
넵 repository에서는 그냥 데이터 저장한다만 담당하고 create 같은 의미있는 기능은 도메인이 들고있어야한다 생각합니다.
요게 밖에서 생성한 RacingGame ID 중복이 발생해서 저장에 실패했을 떄 ID 처리가 어려워서 이렇게한거였는데
생각해보니 save(dto:RacingGameDto) 처럼 전달해서 repository에서 id를 만들도록 하면 되겠네요. 수정하겠습니다 😀
override fun create(): RacingGame {
// 만약 밖에서 RacingGame을 생성해서 전달한다면, id를 어떻게 다시 매핑해주지..?
val createdGame = RacingGame()
val previousValue: RacingGame? = data.putIfAbsent(/* key = */ createdGame.id, /* value = */ createdGame)
if (previousValue != null) {
throw IllegalStateException("같은 ID의 게임이 이미 존재합니다.")
}
return createdGame
}| fun initialize(gameId: GameId, carNames: List<String>, tryCount: Int) { | ||
| val game: RacingGame = racingGameRepository.findById(gameId = gameId) | ||
| ?: throw IllegalArgumentException("게임을 찾을 수 없습니다.") |
There was a problem hiding this comment.
👍 👍 데이터가 없다 그 자체는 repository 의 문제가 아니고, 도메인 영역의 문제죠 👍
| fun play(gameId: GameId): List<RacingGameStatus> { | ||
| val game: RacingGame = racingGameRepository.findById(gameId = gameId) | ||
| ?: throw IllegalArgumentException("게임을 찾을 수 없습니다.") |
There was a problem hiding this comment.
계속해서 반복되므로 이걸 private 메서드로 분리하는 것도 좋아보이네요!
There was a problem hiding this comment.
처음 생성할 때에는 중복으로 보이겠지만,
기능이 조금만 추가되도 각각의 예외처리나 예외 메시지가 달라질 것 같아 private 분리하지 않는게 더 좋아보이긴합니다. private로 분리할 만큼 크게 복잡하지 않은 코드이기도 하구요!
| @JvmInline | ||
| value class GameId(val value: Long) |
| package racingcar.infra | ||
|
|
||
| import racingcar.domain.vo.CarStatus | ||
| import racingcar.domain.vo.RacingGameStatus | ||
| import racingcar.view.MessageCode | ||
| import racingcar.view.RacingGameView | ||
| import racingcar.view.ViewModel | ||
|
|
||
| class StringRacingGameView : RacingGameView { | ||
|
|
||
| override fun getView(racingGameStatus: RacingGameStatus): ViewModel { | ||
| return racingGameStatus.carStatues | ||
| .joinToString(separator = "\n") { "${it.name} : " + "-".repeat(it.position) } | ||
| .let { StringViewModel(it) } | ||
| } |
There was a problem hiding this comment.
infra 패키지에는 domain, view 패키지에 있는 인터페이스들의 구현체가 모두 몰려있네요.
마치 모두가 의존하는 만인의 패키지가 된 느낌인데, domain 을 유지보수 하고 싶을 때도 view 를 유지보수 하고 싶을 때도 infra 를 함께 바라보아야 할거 같아요.
만약 패키지를 기준으로 모듈을 분리한다고 하면, infra 패키지는 분리가 어려울 것 같아요.
지원님은 어떻게 생각하시나요?
There was a problem hiding this comment.
개인적인 의견으론 패키지를 domain, adapter, infra 3개로 쪼개서
domain(interface) <- adapter(Impl) <<- 여러 개의 infra 들(외부의존성, 실제구현) 으로 사용하는게 제일 좋다고 생각해요.
만약 순수 자바코드라면 domain(interface) <- adpater(Impl) 으로 되구요.
만약 패키지를 기준으로 모듈을 분리한다고 하면, infra 패키지는 분리가 어려울 것 같아요.
도메인을 POJO로 추상화 한 것 자체가, 도메인이 어떤것도 의존하지 않게 구성한다는 거고
이건 domain package만 그대로 뜯어가서 로직 재사용이 가능하게 만드는 것과 같다고 생각해요.
adapter(* 지금 코드상에선 infra 패키지)는 한군데 다 몰아놔도 큰 상관 없다고 괜찮다고 보긴합니다.
그러면 모듈 분리할 때 adatper를 하나씩 찾아줘야해서 패키지 분리가 번거롭지 않나? 의 답변은
인프라는 언제든시 교체될 수 있는 의존성이기에, domain 모듈이 분리되어 변경되었다면 그에 맞는 adapter는 다시 작성해주는게 맞지 않나 생각하긴합니다. 그것도 번거롭다면 그냥 adapter 안에 하위 패키지를 만들어서 간단하게만 나눠줘도 괜찮을 거 같네요.
- controller
- ..
- domain
- ..
- adpater
- controller-adapter
- domain-adapter
- other-adapter
No description provided.