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

Step1 지뢰 찾기(그리기) #377

Merged
merged 6 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/main/kotlin/minesweeper/MineSweeper.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package minesweeper

import minesweeper.domain.MineMapGenerator
import minesweeper.domain.MineMapMeta
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)

OutputView.printGameStartMsg()
OutputView.printMineMap(mineMapMeta, mineMap)
}
}

fun main() {
MineSweeper.drawMap()
}
10 changes: 10 additions & 0 deletions src/main/kotlin/minesweeper/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# README

## Step1 구현 리스트
- [x] `InputView` 생성: 사용자로부터 높이와 너비, 지뢰 개수를 입력받고 DTO 객체로 변환한다.
- [x] 지뢰판 생성을 위한 객체들을 구상하고, 관련해 테스트를 구현한다.
- [x] 지뢰판 생성을 위한 객체를 생성한다. (InputView로 받은 DTO 객체 활용)
- [x] 지뢰판 위치 정보를 프로퍼티로 가진 DTO를 생성한다.
- [x] 지뢰와 지뢰가 아닌 칸을 구분하기 위한 문자 및 클래스를 생성한다.
- [x] 지뢰는 주어진 지뢰 개수만큼 랜덤으로 배치한다.
- [x] `OutputView` 생성: 생성된 지뢰판을 출력한다.
9 changes: 9 additions & 0 deletions src/main/kotlin/minesweeper/domain/Cell.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package minesweeper.domain

data class Cell(
val state: CellState
)
Comment on lines +3 to +5
Copy link
Member

Choose a reason for hiding this comment

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

Cell class가 별 다른 역할 없이 CellState 를 가지고만 있는 걸로 보여요.
CellState자체가 Cell로 표현되어도 좋겠어요!

혹은 이후 한 칸에 대한 객체를 더 표현하기 위함이었다면, sealed class를 활용해보셔도 좋겠네요 :)

Copy link
Author

@jaylene-shin jaylene-shin Dec 2, 2023

Choose a reason for hiding this comment

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

칸 객체를 표현할 수 있도록 sealed interface인 Cell 을 생성하고
Mine과 Empty를 하위 클래스로 구현하였습니다!


fun Cell.getStateSymbol(): String {
return this.state.symbol
}
6 changes: 6 additions & 0 deletions src/main/kotlin/minesweeper/domain/CellState.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package minesweeper.domain

enum class CellState(val symbol: String = "") {
MINE("*"),
EMPTY("C");
}
18 changes: 18 additions & 0 deletions src/main/kotlin/minesweeper/domain/MineMap.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package minesweeper.domain

class MineMap(
private val _values: MutableMap<Position, Cell> = mutableMapOf()
) {
val values: Map<Position, Cell>
get() = _values
Comment on lines +3 to +7
Copy link
Member

Choose a reason for hiding this comment

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

지뢰판의 map은 객체 내에서는 가변이지만 외부에서는 불변을 보장할 수 있도록 방어적 복사를 활용하였습니다.

방어적 복사를 활용했다고 보기에는 어려워보여요.
생성자에서 MutableCollection을 받을 때, 다른 개발자가 외부의 참조를 실수로 다뤘을 때, MineMap 객체 내부가 변경되는 상황이 발생할 수 있어요.
아래 코드를 보시면 쉽게 이해하실 수 있을거예요 :)

val mutableMap = mutableMapOf()
val mineMap = MineMap(mutableMapOf())
mutableMap.clear()

생성자에서 전달 받은 컬렉션을 복사해서 멤버변수에 할당하는 것은 어떨까요?

Copy link
Author

@jaylene-shin jaylene-shin Dec 2, 2023

Choose a reason for hiding this comment

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

앗 네네 설명해주셔서 감사합니다..! 방어적 복사는 외부 참조를 끊어내는게 핵심인데 위 코드에서는 끊어내질 못하고 있네요..!
(step2 최종 코드에는 제거되어 있지만) 방어적 복사를 올바르게 활용하도록 수정하였습니다!

class MineMap(
    values: Map<Position, Cell> = mapOf()
) {
    private val _values = values.toMutableMap()
    val values: Map<Position, Cell>
        get() = _values.toMap()
     
    // 이하 생략
}

val size: Int
get() = _values.keys.size

fun plantCell(position: Position, cell: Cell) {
_values[position] = cell
}

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

object MineMapGenerator {
fun generate(minePositions: Positions, emptyPositions: Positions): MineMap {
val mineMap = MineMap()
minePositions.forEach { mineMap.plantCell(it, Cell(CellState.MINE)) }
emptyPositions.forEach { mineMap.plantCell(it, Cell(CellState.EMPTY)) }
Comment on lines +5 to +7
Copy link
Member

Choose a reason for hiding this comment

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

plantCell을 호출함으로써 지뢰판을 초기화 하기보다,
지뢰판을 생성할 때 생성자에 필요한 모든 정보를 넘겨 만들어내는 것은 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋습니다!
plantCell을 활용하다보니 MineMap의 프로퍼티를 mutable로 써야했었는데요. 이 문제를 같이 해결할 수 있을 것 같습니다..!
피드백 주신 방향으로 수정 완료했습니다!

return mineMap
}
}
18 changes: 18 additions & 0 deletions src/main/kotlin/minesweeper/domain/MineMapMeta.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package minesweeper.domain

data class MineMapMeta(
val height: Int,
val width: Int,
val mineCount: Int
) {
init {
require(height > 0) { "높이는 0 이거나 음수일 수 없습니다" }
require(width > 0) { "너비는 0 이거나 음수일 수 없습니다" }
require(mineCount > 0) { "지뢰 개수는 0 이거나 음수일 수 없습니다" }
require(height * width >= mineCount) { "지뢰 개수는 (높이 x 너비) 개수를 초과할 수 없습니다" }
Comment on lines +9 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.

어떤 값이 들어와서 예외가 발생했는지 입력값도 메시지에 같이 표시해주면 디버깅할 때 더욱 도움이 될 것으로 기대돼요 :)

Copy link
Author

Choose a reason for hiding this comment

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

트러블슈팅에 훨씬 유리하겠네요..! 수정하였습니다!

}

fun getCellCount(): Int {
return height * width
}
}
11 changes: 11 additions & 0 deletions src/main/kotlin/minesweeper/domain/Position.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package minesweeper.domain

data class Position(
val y: Int,
val x: Int
) {
init {
require(y > 0) { "y는 0이거나 음수일 수 없습니다" }
require(x > 0) { "x는 0이거나 음수일 수 없습니다" }
}
}
35 changes: 35 additions & 0 deletions src/main/kotlin/minesweeper/domain/PositionGenerator.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package minesweeper.domain

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)
}
}
Comment on lines +7 to +17
Copy link
Member

Choose a reason for hiding this comment

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

지뢰가 위치할 (랜덤) 좌표와 빈 좌표 정보를 각각 생성하여 지뢰판을 구성하였습니다. 현재 방식이 지뢰판 생성 중 발생 가능한 반복 탐색(e.g. 지뢰판의 모든 좌표가 랜덤하게 생성된 지뢰 좌표와 중복되는지 탐색. O(height*width))을 최소화하는 방식이라고 판단하였는데요...! 옳은 판단인지 피드백을 받고 싶습니다!

운이 좋지 않으면 굉장히 많이 실행되어서 시간이 꽤 걸릴지도 모르겠어요.

아래 형태로 접근 방식을 바꿔보는 건 어떨까요?
로직적인 것만 참고해주시고, 이름은 지영님이 잘 지어보시면 좋겠어요 :)

val allPosition = ~~
val minePosition = RandomSelector.select(allPosition, 개수) // 전달 받은 positions을 shuffle하고, 개수만큼 take() 한positions을 반환
val cells = minePosition.map { Mine() } + (allPosition - minPosition).map { Block() }
~~

Copy link
Author

Choose a reason for hiding this comment

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

재귀 써야한다는 생각에 빠져있었는데, shuffle-take 연산이 훨씬 효율적이겠네요..!! 감사합니다!
수정하였습니다!


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()
}
}
5 changes: 5 additions & 0 deletions src/main/kotlin/minesweeper/domain/PositionSelector.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package minesweeper.domain

interface PositionSelector {
fun select(mineMapMeta: MineMapMeta): Position
}
14 changes: 14 additions & 0 deletions src/main/kotlin/minesweeper/domain/Positions.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package minesweeper.domain

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

Choose a reason for hiding this comment

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

일급 컬렉션을 kotlin delegate를 활용해서 작성해주셨네요!
delegate를 활용하면 외부에서 활용하지 않아도 될 컬렉션의 api 조차도 모두 공개되기 때문에 일급 컬렉션을 만드는 의미가 희석된다고 생각해요.
필요한 기능들만 외부에 공개해보는 건 어떨까요?

사용하는 것은 편리해질 수 있겠지만, Collection을 래핑만 한 것과 크게 다르지 않다는 생각이에요 :)

Copy link
Author

Choose a reason for hiding this comment

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

기존에는 내부 프로퍼티를 바로 순회할 수 있게 delegate을 사용했었는데요.
불필요한 컬렉션 api 를 모두 공개한다면, 일급 컬렉션의모든 행위를 내부에서 관리한다는 목적이 희석될 수 있겠네요...!
좋은 피드백 감사합니다. 👍
delegate 제거하도록 수정하였습니다!

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

operator fun plus(position: Position): Positions = Positions(this.positions + position)
operator fun plus(positions: Positions): Positions = Positions(this.positions + positions.positions)
operator fun minus(position: Position): Positions = Positions(this.positions - position)
operator fun minus(positions: Positions): Positions = Positions(this.positions - positions.positions)
}

fun Set<Position>.toPositions(): Positions = Positions(this)
13 changes: 13 additions & 0 deletions src/main/kotlin/minesweeper/domain/RandomPositionSelector.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
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)
}
}
21 changes: 21 additions & 0 deletions src/main/kotlin/minesweeper/view/InputView.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package minesweeper.view

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

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

fun getMineCount(): Int {
return getNumber("지뢰는 몇 개인가요?")
}

private fun getNumber(consoleMsg: String): Int {
println(consoleMsg)
val input = readlnOrNull() ?: throw IllegalArgumentException("입력값은 공백이거나 빈 문자열일 수 없습니다.")
return input.toIntOrNull() ?: throw IllegalArgumentException("숫자가 아닌 입력값은 들어올 수 없습니다.")
}
}
25 changes: 25 additions & 0 deletions src/main/kotlin/minesweeper/view/OutputView.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package minesweeper.view

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

object OutputView {
fun printGameStartMsg() {
println("\n지뢰 찾기 게임 시작")
}

fun printMineMap(mineMapMeta: MineMapMeta, mineMap: MineMap) {
for (row in 1 until mineMapMeta.height + 1) {
printRowCells(mineMapMeta, mineMap, row)
}
}

private fun printRowCells(mineMapMeta: MineMapMeta, mineMap: MineMap, row: Int) {
for (col in 1 until mineMapMeta.width + 1) {
print(mineMap.getCell(Position(row, col)).getStateSymbol() + " ")
}
println()
}
}
31 changes: 31 additions & 0 deletions src/test/kotlin/minesweeper/domain/MineMapGeneratorTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package minesweeper.domain

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test

class MineMapGeneratorTest {
@Test
fun `지뢰 맵을 구성할 위치 정보가 주어졌을 때 MineMap을 생성할 수 있다`() {
// given
val mineMapMeta = MineMapMeta(3, 3, 3)
val minePositions = setOf(
Position(1, 1),
Position(2, 2),
Position(3, 3)
).toPositions()
val emptyPositions = setOf(
Position(1, 2),
Position(1, 3),
Position(2, 1),
Position(2, 3),
Position(3, 1),
Position(3, 2)
).toPositions()

// when
val mineMap = MineMapGenerator.generate(minePositions, emptyPositions)

// then
assertEquals(mineMapMeta.getCellCount(), mineMap.values.size)
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

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

테스트 검증문은 assertJ 혹은 kotest를 활용해보는 건 어떨까요?

Copy link
Author

@jaylene-shin jaylene-shin Dec 2, 2023

Choose a reason for hiding this comment

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

가독성 확보와 메서드 체이닝 활용 측면에서 더 유리할 것 같습니다!
step2에서는 assertJ의 assertThat().isEqualTo()... 방식으로 수정하였습니다! (예시)

}
}
69 changes: 69 additions & 0 deletions src/test/kotlin/minesweeper/domain/MineMapMetaTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package minesweeper.domain

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.CsvSource

class MineMapMetaTest {
@ParameterizedTest
@CsvSource(
"0 0 0",
"1 0 0",
"1 1 -1",
"0 -1 -1"
)
fun `MineMapMeta 프로퍼티는 0이거나 음수일 수 없다`(input: String) {
// given
val properties = input.split(" ").map { it.toInt() }

assertThrows<IllegalArgumentException> { // then
MineMapMeta( // when
height = properties[0],
width = properties[1],
mineCount = properties[2]
)
}
}

@ParameterizedTest
@CsvSource(
"1 1 2",
"10 1 11",
"10 10 101"
)
fun `지뢰 개수는 MineMap을 구성하는 셀 크기(height*width)를 초과할 수 없다`(input: String) {
// given
val properties = input.split(" ").map { it.toInt() }

assertThrows<IllegalArgumentException> { // then
MineMapMeta( // when
height = properties[0],
width = properties[1],
mineCount = properties[2]
)
}
}

@ParameterizedTest
@CsvSource(
"1 1 1, 1",
"1 10 5, 10",
"10 30 50, 300"
)
fun `MineMap을 구성하는 Cell 개수를 반환한다`(input: String, expected: Int) {
// given
val properties = input.split(" ").map { it.toInt() }
val mineMapMeta = MineMapMeta(
height = properties[0],
width = properties[1],
mineCount = properties[2]
)

// when
val cellCount = mineMapMeta.getCellCount()

// then
assertEquals(expected, cellCount)
}
}
59 changes: 59 additions & 0 deletions src/test/kotlin/minesweeper/domain/MineMapTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package minesweeper.domain

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows

class MineMapTest {
@Test
fun `위치 정보를 전달해 MineMap에 지뢰를 심는다`() {
// given
val mineMap = MineMap()

// when
mineMap.plantCell(Position(1, 1), Cell(CellState.MINE))

// then
assertEquals(1, mineMap.size)
assertEquals(CellState.MINE, mineMap.values[Position(1, 1)]?.state)
}

@Test
fun `위치 정보를 전달해 MineMap에 빈 상태를 심는다`() {
// given
val mineMap = MineMap()

// when
mineMap.plantCell(Position(1, 1), Cell(CellState.EMPTY))
mineMap.plantCell(Position(2, 2), Cell(CellState.EMPTY))

// then
assertEquals(2, mineMap.size)
assertEquals(CellState.EMPTY, mineMap.values[Position(1, 1)]?.state)
assertEquals(CellState.EMPTY, mineMap.values[Position(2, 2)]?.state)
Comment on lines +30 to +33
Copy link
Member

Choose a reason for hiding this comment

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

assert 구문을 나열하는 것과 assertAll로 감싸는 것은 어떤 차이점이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

assert 구문 나열 시 실패한 구문 이후로 테스트가 수행되지 않을 것 같습니다..!
assertSoftly를 도입해 위 문제를 해결하였습니다!

}

@Test
fun `주어진 위치 정보에 놓인 Cell 객체를 가져온다`() {
// given
val mineMap = MineMap()
val position = Position(1, 1)
mineMap.plantCell(position, Cell(CellState.EMPTY))

// when
val cell = mineMap.getCell(position)

// then
assertEquals(CellState.EMPTY, cell.state)
}

@Test
fun `MineMap에 없는 위치 정보를 통해 Cell 객체를 가져온다면 IllegalArgumentException이 발생한다`() {
// given
val mineMap = MineMap()

assertThrows<IllegalArgumentException> { // then
mineMap.getCell(Position(1, 1)) // when
}
}
}
Loading