-
Notifications
You must be signed in to change notification settings - Fork 206
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
Step2 지뢰 찾기(지뢰 개수) #389
Changes from all commits
f90e381
f47cb95
ed3dfd9
7ebd075
141534e
09bbe51
adccb26
01fb40e
26b131c
9fa9112
1e6ca41
ce01b91
6595eb3
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 |
---|---|---|
@@ -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 |
This file was deleted.
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) | ||
} | ||
} | ||
} |
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("해당 위치에 셀이 없습니다") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package minesweeper.domain | ||
|
||
interface MineMapFactory { | ||
fun create(): MineMap | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
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. 항상 require를 충족할 불필요한 검증문이 맞아 제거하였습니다! (질문) 만약 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. 테스트만을 위해서 메서드를 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() | ||
} | ||
} |
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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,14 @@ package minesweeper.domain | |
|
||
class Positions( | ||
private val positions: Set<Position> = emptySet() | ||
Comment on lines
3
to
4
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. Positions 객체가 특별히 수행하는 역할을 가지고 있지 않아보여요. 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. 특별한 비즈니스 로직을 수행하거나 유지보수 용이성이 없는 코드로 보여 언래핑하였습니다! |
||
) : 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) | ||
|
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() | ||
} | ||
} |
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
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. FixedPositionSelector가 어떤 값을 반환하는지는 직접 구현체를 보지 않고는 알 수 없을 거라 생각해요. 테스트 코드만을 보더라도, 어떤 값을 반환하는지 한 눈에 볼 수 있을것으로 기대돼요 :) 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. 현재 아래와 같은 주입 구조를 가지고 있어 MineCountMapFactory가 PositionSelector로부터 받은 위치 정보를 바로 활용할 수는 없습니다ㅠ
객체 의존 관계가 복잡해질 수록 테스트하기 어렵거나 명확한 의미 전달이 어려워지는데요..! 의존도를 낮추기 위해 객체 협력 관계를 다시 정의해야 할지 고민이 됩니다 😭 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. 해당 주입 구조를 가지고 있기에 충분히 테스트 가능해 보여요! 제가 드린 제안은, FixedPositionSelector처럼 직접 상속해서 구현하기보다, 람다로 만들어서 넘겨보시라는 의미었어요. |
||
|
||
@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
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. 좋은 아이디어 감사합니다! 반영했습니다! |
||
} |
This file was deleted.
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.
특정 위치의 인접 8방향에 대한 내용을 도메인 객체로 표현해보는건 어떨까요?
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.
또, 인접한 칸을 찾는 과정에서, out of index 이슈를 회피하기 위해, 지뢰판 가장 바깥에 아무 역할도 수행하지 않는 칸을 두어서 지뢰판을 구성해볼 수도 있어요 :)
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.
좋은 아이디어 같습니다 :) 반영했습니다!