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

Step2 지뢰 찾기(지뢰 개수) #389

Merged
merged 13 commits into from
Dec 3, 2023
Merged
15 changes: 3 additions & 12 deletions src/main/kotlin/minesweeper/MineSweeper.kt
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
package minesweeper

import minesweeper.domain.MineMapGenerator
import minesweeper.domain.MineMapMeta
import minesweeper.domain.MineCountMapFactory
import minesweeper.domain.PositionGenerator
import minesweeper.view.InputView
import minesweeper.view.OutputView

object MineSweeper {
fun drawMap() {
val mineMapMeta = MineMapMeta(
height = InputView.getHeight(),
width = InputView.getWidth(),
mineCount = InputView.getMineCount()
)
val positionGenerator = PositionGenerator(mineMapMeta)
val minePositions = positionGenerator.generateMinePositions()
val emptyPositions = positionGenerator.generateEmptyPositions(minePositions)
val mineMap = MineMapGenerator.generate(minePositions, emptyPositions)

val mineMapMeta = InputView.readMineMapMeta()
val mineMap = MineCountMapFactory(PositionGenerator(mineMapMeta)).create()
OutputView.printGameStartMsg()
OutputView.printMineMap(mineMapMeta, mineMap)
}
Expand Down
9 changes: 3 additions & 6 deletions src/main/kotlin/minesweeper/domain/Cell.kt
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package minesweeper.domain

data class Cell(
val state: CellState
)
sealed interface Cell

fun Cell.getStateSymbol(): String {
return this.state.symbol
}
object Mine : Cell
data class Empty(val mineCount: Int = 0) : Cell
6 changes: 0 additions & 6 deletions src/main/kotlin/minesweeper/domain/CellState.kt

This file was deleted.

24 changes: 24 additions & 0 deletions src/main/kotlin/minesweeper/domain/MineCountMapFactory.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package minesweeper.domain

class MineCountMapFactory(
private val positionGenerator: PositionGenerator
) : MineMapFactory {
override fun create(): MineMap {
val minePositions = positionGenerator.generateMinePositions()
val emptyPositions = positionGenerator.generateEmptyPositions(minePositions)
val cells = (minePositions + emptyPositions)
.getValues()
.associateWith { createCell(it, minePositions) }
return MineMap(cells)
}

private fun createCell(position: Position, minePositions: Positions): Cell {
val aroundPositions = position.aroundPositions()
return if (minePositions.contains(position)) {
Mine
} else {
val mineCount = aroundPositions.count { minePositions.contains(it) }
Empty(mineCount)
}
}
}
12 changes: 3 additions & 9 deletions src/main/kotlin/minesweeper/domain/MineMap.kt
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
package minesweeper.domain

class MineMap(
private val _values: MutableMap<Position, Cell> = mutableMapOf()
private val values: Map<Position, Cell>
) {
val values: Map<Position, Cell>
get() = _values
val size: Int
get() = _values.keys.size

fun plantCell(position: Position, cell: Cell) {
_values[position] = cell
}
get() = values.keys.size

fun getCell(position: Position): Cell {
return _values[position] ?: throw IllegalArgumentException("해당 위치에 셀이 없습니다")
return values[position] ?: throw IllegalArgumentException("해당 위치에 셀이 없습니다")
}
}
5 changes: 5 additions & 0 deletions src/main/kotlin/minesweeper/domain/MineMapFactory.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package minesweeper.domain

interface MineMapFactory {
fun create(): MineMap
}
10 changes: 0 additions & 10 deletions src/main/kotlin/minesweeper/domain/MineMapGenerator.kt

This file was deleted.

8 changes: 4 additions & 4 deletions src/main/kotlin/minesweeper/domain/MineMapMeta.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ data class MineMapMeta(
val mineCount: Int
) {
init {
require(height > 0) { "높이는 0 이거나 음수일 수 없습니다" }
require(width > 0) { "너비는 0 이거나 음수일 수 없습니다" }
require(mineCount > 0) { "지뢰 개수는 0 이거나 음수일 수 없습니다" }
require(height * width >= mineCount) { "지뢰 개수는 (높이 x 너비) 개수를 초과할 수 없습니다" }
require(height > 0) { "입력값: $height, 높이는 0 이거나 음수일 수 없습니다" }
require(width > 0) { "입력값: $width, 너비는 0 이거나 음수일 수 없습니다" }
require(mineCount > 0) { "입력값: $mineCount, 지뢰 개수는 0 이거나 음수일 수 없습니다" }
require(height * width >= mineCount) { "입력값: ${height * width}, 지뢰 개수는 (높이 x 너비) 개수를 초과할 수 없습니다" }
}

fun getCellCount(): Int {
Expand Down
22 changes: 20 additions & 2 deletions src/main/kotlin/minesweeper/domain/Position.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,25 @@ data class Position(
val x: Int
) {
init {
require(y > 0) { "y는 0이거나 음수일 수 없습니다" }
require(x > 0) { "x는 0이거나 음수일 수 없습니다" }
require(y > 0) { "입력값: $y, y는 0이거나 음수일 수 없습니다" }
require(x > 0) { "입력값: $x, x는 0이거나 음수일 수 없습니다" }
}

fun aroundPositions(): List<Position> {
return listOfNotNull(
topOrNull(),
bottomOrNull(),
leftOrNull(),
rightOrNull(),
topOrNull()?.leftOrNull(),
topOrNull()?.rightOrNull(),
bottomOrNull()?.leftOrNull(),
bottomOrNull()?.rightOrNull()
)
}

private fun leftOrNull(): Position? = runCatching { Position(y, x - 1) }.getOrNull()
private fun rightOrNull(): Position? = runCatching { Position(y, x + 1) }.getOrNull()
private fun topOrNull(): Position? = runCatching { Position(y - 1, x) }.getOrNull()
private fun bottomOrNull(): Position? = runCatching { Position(y + 1, x) }.getOrNull()
Comment on lines +13 to +28
Copy link
Member

Choose a reason for hiding this comment

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

특정 위치의 인접 8방향에 대한 내용을 도메인 객체로 표현해보는건 어떨까요?

Copy link
Member

Choose a reason for hiding this comment

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

또, 인접한 칸을 찾는 과정에서, out of index 이슈를 회피하기 위해, 지뢰판 가장 바깥에 아무 역할도 수행하지 않는 칸을 두어서 지뢰판을 구성해볼 수도 있어요 :)

Copy link
Author

Choose a reason for hiding this comment

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

좋은 아이디어 같습니다 :) 반영했습니다!

}
34 changes: 15 additions & 19 deletions src/main/kotlin/minesweeper/domain/PositionGenerator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,28 @@ class PositionGenerator(
private val mineMapMeta: MineMapMeta,
private val positionSelector: PositionSelector = RandomPositionSelector
) {
tailrec fun generateMinePositions(
minePositions: Positions = Positions()
): Positions {
if (minePositions.size == mineMapMeta.mineCount) { return minePositions }
val randomMinePosition = positionSelector.select(mineMapMeta)
return if (randomMinePosition in minePositions) {
generateMinePositions(minePositions)
} else {
generateMinePositions(minePositions + randomMinePosition)
}
private val allPositions = generateAllPositions()

private fun generateAllPositions(): Positions {
val allPositions = (1..mineMapMeta.height)
.flatMap { y -> (1..mineMapMeta.width).map { x -> Position(y, x) } }
.toSet()
.toPositions()
require(allPositions.size == mineMapMeta.getCellCount()) { "모든 위치를 생성하지 못했습니다" }
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

불필요한 검증문이라 생각해요.
오히려 mineMapMeta.getCellCount() 변경점에 대해 불필요한 의존관계를 가지고 있다는 생각도 들어요.
getCellCount()를 리팩터링 하면서 실수로 잘못만들었을 때 엉뚱한 녀석이 에러를 뱉어 빠른 디버깅을 방해할지도 모른다는 생각이 드네요!

오히려 이런 부분은 테스트코드로 검증해보는 게 의도를 더 잘 나타낼 수 있는 방법이라 생각해요 :)

Copy link
Author

Choose a reason for hiding this comment

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

항상 require를 충족할 불필요한 검증문이 맞아 제거하였습니다!


(질문) 만약 generateAllPositions 과 같은 private 함수를 테스트하려면 public으로 공개해야 할까요? 테스트를 위해 접근 제어자를 변경해야 할지 고민입니다..! 🤔

Copy link
Member

Choose a reason for hiding this comment

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

테스트만을 위해서 메서드를 public으로 열 필요는 없습니다.
private 메서드는 결국 public 메서드로 인해 호출되기에 간접적으로 테스트할 수 있다고 생각해 볼 수 있어요 :)

return allPositions
}

fun generateMinePositions(): Positions {
val minePositions = positionSelector.select(allPositions, mineMapMeta.mineCount)
require(minePositions.size == mineMapMeta.mineCount) { "지뢰의 개수가 맞지 않습니다." }
return minePositions
}

fun generateEmptyPositions(
minePositions: Positions
): Positions {
val allPositions = generateAllPositions()
require(allPositions.size == mineMapMeta.getCellCount()) { "모든 위치를 생성하지 못했습니다" }
val emptyPositions = allPositions - minePositions
require(!emptyPositions.containSamePosition(minePositions)) { "지뢰와 빈 공간은 겹칠 수 없습니다." }
return emptyPositions
}

private fun generateAllPositions(): Positions {
return (1..mineMapMeta.height)
.flatMap { y -> (1..mineMapMeta.width).map { x -> Position(y, x) } }
.toSet()
.toPositions()
}
}
2 changes: 1 addition & 1 deletion src/main/kotlin/minesweeper/domain/PositionSelector.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package minesweeper.domain

interface PositionSelector {
fun select(mineMapMeta: MineMapMeta): Position
fun select(positions: Positions, selectNum: Int): Positions
}
10 changes: 8 additions & 2 deletions src/main/kotlin/minesweeper/domain/Positions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ package minesweeper.domain

class Positions(
private val positions: Set<Position> = emptySet()
Comment on lines 3 to 4
Copy link
Member

Choose a reason for hiding this comment

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

Positions 객체가 특별히 수행하는 역할을 가지고 있지 않아보여요.
Positions를 반환하는 곳에서 Set을 반환하더라도 무방해보여요.
Positions가 갖고있는 컬렉션으로 도메인 역할을 수행하게 만들거나, 혹은 언래핑 해보는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

특별한 비즈니스 로직을 수행하거나 유지보수 용이성이 없는 코드로 보여 언래핑하였습니다!

) : Set<Position> by positions {
infix fun containSamePosition(otherPositions: Positions): Boolean = positions.intersect(otherPositions).isNotEmpty()
) {
val size
get() = positions.size

fun getValues(): Set<Position> = positions
fun contains(position: Position): Boolean = positions.contains(position)

infix fun containSamePosition(otherPositions: Positions): Boolean = positions.intersect(otherPositions.getValues()).isNotEmpty()

operator fun plus(position: Position): Positions = Positions(this.positions + position)
operator fun plus(positions: Positions): Positions = Positions(this.positions + positions.positions)
Expand Down
15 changes: 7 additions & 8 deletions src/main/kotlin/minesweeper/domain/RandomPositionSelector.kt
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package minesweeper.domain

import kotlin.random.Random

object RandomPositionSelector : PositionSelector {
override fun select(
mineMapMeta: MineMapMeta
): Position {
val x = Random.nextInt(1, mineMapMeta.width + 1)
val y = Random.nextInt(1, mineMapMeta.height + 1)
return Position(y, x)
override fun select(positions: Positions, selectNum: Int): Positions {
return positions
.getValues()
.shuffled()
.take(selectNum)
.toSet()
.toPositions()
}
}
21 changes: 11 additions & 10 deletions src/main/kotlin/minesweeper/view/InputView.kt
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package minesweeper.view

object InputView {
fun getHeight(): Int {
return getNumber("높이를 입력하세요.")
}
import minesweeper.domain.MineMapMeta

fun getWidth(): Int {
return getNumber("너비를 입력하세요.")
}

fun getMineCount(): Int {
return getNumber("지뢰는 몇 개인가요?")
object InputView {
fun readMineMapMeta(): MineMapMeta {
val height = getNumber("높이를 입력하세요.")
val width = getNumber("너비를 입력하세요.")
val mineCount = getNumber("지뢰는 몇 개인가요?")
return MineMapMeta(
height = height,
width = width,
mineCount = mineCount
)
}

private fun getNumber(consoleMsg: String): Int {
Expand Down
10 changes: 8 additions & 2 deletions src/main/kotlin/minesweeper/view/OutputView.kt
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package minesweeper.view

import minesweeper.domain.Empty
import minesweeper.domain.Mine
import minesweeper.domain.MineMap
import minesweeper.domain.MineMapMeta
import minesweeper.domain.Position
import minesweeper.domain.getStateSymbol

object OutputView {
private const val MINE_CHAR = "*"

fun printGameStartMsg() {
println("\n지뢰 찾기 게임 시작")
}
Expand All @@ -18,7 +21,10 @@ object OutputView {

private fun printRowCells(mineMapMeta: MineMapMeta, mineMap: MineMap, row: Int) {
for (col in 1 until mineMapMeta.width + 1) {
print(mineMap.getCell(Position(row, col)).getStateSymbol() + " ")
when (val cell = mineMap.getCell(Position(row, col))) {
is Mine -> print("$MINE_CHAR ")
is Empty -> print("${cell.mineCount} ")
}
}
println()
}
Expand Down
36 changes: 36 additions & 0 deletions src/test/kotlin/minesweeper/domain/MineCountMapFactoryTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package minesweeper.domain

import io.kotest.assertions.assertSoftly
import minesweeper.domain.support.FixedPositionSelector
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

class MineCountMapFactoryTest {
private val positionGenerator = PositionGenerator(
MineMapMeta(3, 3, 3),
FixedPositionSelector
)
Comment on lines +11 to +12
Copy link
Member

Choose a reason for hiding this comment

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

FixedPositionSelector가 어떤 값을 반환하는지는 직접 구현체를 보지 않고는 알 수 없을 거라 생각해요.
테스트 코드에서 바로 람다로 원하는 위치를 반환하게 명시해보는 건 어떨까요?

테스트 코드만을 보더라도, 어떤 값을 반환하는지 한 눈에 볼 수 있을것으로 기대돼요 :)

Copy link
Author

@jaylene-shin jaylene-shin Dec 3, 2023

Choose a reason for hiding this comment

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

현재 아래와 같은 주입 구조를 가지고 있어 MineCountMapFactory가 PositionSelector로부터 받은 위치 정보를 바로 활용할 수는 없습니다ㅠ

(MineCountMapFactory <- PositionGenerator <- PositionSelector)

객체 의존 관계가 복잡해질 수록 테스트하기 어렵거나 명확한 의미 전달이 어려워지는데요..! 의존도를 낮추기 위해 객체 협력 관계를 다시 정의해야 할지 고민이 됩니다 😭

Copy link
Member

Choose a reason for hiding this comment

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

해당 주입 구조를 가지고 있기에 충분히 테스트 가능해 보여요!
FixedPositionSelector를 넣을 수 있는 것이 테스트 가능함을 암시한다고 볼 수 있겠어요 :)

제가 드린 제안은, FixedPositionSelector처럼 직접 상속해서 구현하기보다, 람다로 만들어서 넘겨보시라는 의미었어요.
해당 interface가 람다로 변환이 되지 않는것이 문제였다면,
fun interface에 대해 학습해보셔도 좋겠어요 :)


@Test
fun `주변 지뢰 개수를 담고 있는 cell을 생성해 Map을 구성한다`() {
// given
val mineCountMapFactory = MineCountMapFactory(positionGenerator)

// when
val mineMap = mineCountMapFactory.create()

// then
assertSoftly {
assertThat(mineMap.size).isEqualTo(9)
assertThat(mineMap.getCell(Position(1, 1))).usingRecursiveComparison().isEqualTo(Mine)
assertThat(mineMap.getCell(Position(1, 2))).usingRecursiveComparison().isEqualTo(Mine)
assertThat(mineMap.getCell(Position(1, 3))).usingRecursiveComparison().isEqualTo(Mine)
assertThat(mineMap.getCell(Position(2, 1))).usingRecursiveComparison().isEqualTo(Empty(2))
assertThat(mineMap.getCell(Position(2, 2))).usingRecursiveComparison().isEqualTo(Empty(3))
assertThat(mineMap.getCell(Position(2, 3))).usingRecursiveComparison().isEqualTo(Empty(2))
assertThat(mineMap.getCell(Position(3, 1))).usingRecursiveComparison().isEqualTo(Empty(0))
assertThat(mineMap.getCell(Position(3, 2))).usingRecursiveComparison().isEqualTo(Empty(0))
assertThat(mineMap.getCell(Position(3, 3))).usingRecursiveComparison().isEqualTo(Empty(0))
}
}
Comment on lines +22 to +35
Copy link
Member

Choose a reason for hiding this comment

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

어떤 지뢰판인지 주석을 활용해서 시각적으로 표현해보는 건 어떨까요?
테스트를 더욱 가독성있게 작성할 수 있도록 도와줍니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

좋은 아이디어 감사합니다! 반영했습니다!

}
31 changes: 0 additions & 31 deletions src/test/kotlin/minesweeper/domain/MineMapGeneratorTest.kt

This file was deleted.

Loading