Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Step3: 지뢰 찾기(게임 실행) #394

Open
wants to merge 12 commits into
base: factoriall
Choose a base branch
from

Conversation

Factoriall
Copy link

3단계 진행했습니다. 코멘트를 달겠습니다!

Comment on lines +16 to 38
/**
* 테스트 내에서만 쓰는, 특정 형태의 string을 입력 시 MineMap을 뽑아주는 메소드
*
* ex)
* ...*
* ....
* .*..
* ....
*
* -> 4x4 맵에 (1열 4행), (3열 2행) 지뢰가 있음
*/
private fun createMap(mapString: String): MineMap {
val rows = mapString.split("\n")

val mines = mutableListOf<Point>()
var mineCount = 0
rows.forEachIndexed { row, str ->
val columnList = str.withIndex().filter { it.value == '*' }.map { it.index }
for (col in columnList) {
mines.add(Point(row + 1, col + 1))
mineCount++
}
}
Copy link
Author

Choose a reason for hiding this comment

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

테스트 코드 내에서만 쓰이는, 가짜 생성자를 만들었습니다. 이를 통해 시각적인 효과를 보여줄 수 있을 거라 생각합니다.

ClickResult.CONTINUE
}

else -> ClickResult.ERROR
Copy link
Author

Choose a reason for hiding this comment

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

사실 여기 들어가면 안되는데, Map 특성상 null이 될 확률을 컴파일러 내에서 배제하기 힘들어서 ERROR를 따로 넣었습니다. 이를 없앨 수 있는 좋은 방법이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

mineMap.mineMap[point]이 null이 되는 건 게임을 진행하면서 결코 피할 수 없을 거라 생각해요.

오히려 게임 진행에 있어서 필수적인 흐름이란 생각이 드는데요,
이런 경우가 어떨 때 생기는지 ClickResult.ERROR의 이름을 변경함으로써 표현하고, 게임 진행에서 어떻게 처리하면 좋을지 고민해보셔도 좋겠어요!

Comment on lines +33 to +48
private fun setAdjacentTilesClicked(point: Point) {
val adjacent = point.getAdjacentPoints(mineMap.mapInfo.mapSize)
for (adj in adjacent) {
if (clickedSet.contains(adj)) continue

adjacentTileDfs(adj)
}
}

private fun adjacentTileDfs(adj: Point) {
val info = mineMap.mineMap[adj]
if (info is MapTile.Blank) {
setClickedPoint(adj)
if (info.isNoMineNear) setAdjacentTilesClicked(adj)
}
}
Copy link
Author

Choose a reason for hiding this comment

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

여기 DFS 구현한다고 했는데... 사실 이름이 크게 떠오르지 않아서 이상한 메소드를 쓴 것 같습니다. 혹시 네이밍 관련 좋은 이름이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

수행하고자 하는 것이 무엇인지를 고민해서 메서드명에 녹여내보면 좋겠어요 :)
정답은 없으니 충분히 고민해보는 시간을 가져보셔도 좋겠어요.

Comment on lines -1 to -30
package minesweeper

class AdjacentPoints private constructor(val points: List<Point>) {
companion object {
fun create(center: Point, mapRow: Int, mapCol: Int) =
AdjacentPoints(
buildList {
for (dir in Direction.values()) {
val nearPoint = Point(center.row + dir.row, center.col + dir.col)
if (nearPoint.isOutOfBound(mapRow, mapCol)) continue
add(nearPoint)
}
}
)

private fun Point.isOutOfBound(mapRow: Int, mapCol: Int): Boolean =
this.row < 0 || this.col < 0 || this.row >= mapRow || this.col >= mapCol
}

enum class Direction(val row: Int, val col: Int) {
NORTH(-1, 0),
NORTHEAST(-1, 1),
EAST(0, 1),
SOUTHEAST(1, 1),
SOUTH(1, 0),
SOUTHWEST(1, -1),
WEST(0, -1),
NORTHWEST(-1, -1)
}
}
Copy link
Author

Choose a reason for hiding this comment

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

Point 내에 메시지를 던지게 만듦으로써 필요없어진 클래스를 삭제했습니다.

@@ -3,7 +3,7 @@ package minesweeper
@JvmInline
value class MineCount(val count: Int) {
init {
require(count > 0) {
require(count >= 0) {
Copy link
Author

Choose a reason for hiding this comment

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

이건 지뢰가 0개일 수도 있을거 같다는 생각에 바꿨습니다.

Comment on lines +32 to +38
return mutableMapOf<Point, MapTile>().apply {
for (i in 1..mapSize.row.count) {
for (j in 1..mapSize.column.count) {
put(Point(i, j), MapTile.Blank(0))
}
}
}
Copy link
Author

Choose a reason for hiding this comment

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

Blank(0)인 Map을 초기화 시 넣게 했습니다.

private fun Int.toPoint(rowNum: Int): Point {
val rowIdx = this / rowNum
val colIdx = this % rowNum
return Point(rowIdx + 1, colIdx + 1)
Copy link
Author

Choose a reason for hiding this comment

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

게임 실행을 살펴보니 1,1부터 시작하는 걸로 보여서 +1 붙여줬습니다.

Copy link
Member

@malibinYun malibinYun left a comment

Choose a reason for hiding this comment

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

3단계 미션 고생 많으셨어요 :)
고민해보면 좋을 점들에 코멘트 남겨두었어요.
피드백 반영 후 다시 리뷰 요청 부탁드려요!

Comment on lines 3 to 6
class MineMap(
val mineMapInfo: MineMapInfo,
createStrategy: MinePointCreateStrategy = RandomPointCreateStrategy()
val mineMap: Map<Point, MapTile>,
val mapInfo: MineMapInfo
) {
Copy link
Member

Choose a reason for hiding this comment

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

MineMap이 단순히 Map 컬렉션을 가지고있는 것 말고 큰 역할을 수행하고 있지 않네요!
이 객체가 수행해야할 역할을 모아보는건 어떨까요?

Comment on lines +41 to +46
private fun createNear(map: MutableMap<Point, MapTile>, mine: Point, mapSize: MapSize) {
val adjacentPoints = mine.getAdjacentPoints(mapSize)
for (adj in adjacentPoints) {
val nearInfo = map[adj]
if (nearInfo is MapTile.Blank) map[adj] = nearInfo + 1
}
Copy link
Member

Choose a reason for hiding this comment

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

지뢰 찾기를 진행하면서, 칸을 눌렀을 때, 칸의 숫자를 계산하도록 만드는 건 어떨까요?
게임을 진행하면서 열리지 않을 칸들에 대해, 생성 시점에서 주변 지뢰 수를 계산하는 것은 낭비라는 생각이 드네요.

Comment on lines +5 to +9
clickedSet: Set<Point> = setOf(),
) {
var clickedSet: Set<Point> = clickedSet
private set

Copy link
Member

Choose a reason for hiding this comment

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

MineTile의 하위 구현체로, 열린 칸과 닫힌 칸을 표현해보는 건 어떨까요?

ClickResult.CONTINUE
}

else -> ClickResult.ERROR
Copy link
Member

Choose a reason for hiding this comment

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

mineMap.mineMap[point]이 null이 되는 건 게임을 진행하면서 결코 피할 수 없을 거라 생각해요.

오히려 게임 진행에 있어서 필수적인 흐름이란 생각이 드는데요,
이런 경우가 어떨 때 생기는지 ClickResult.ERROR의 이름을 변경함으로써 표현하고, 게임 진행에서 어떻게 처리하면 좋을지 고민해보셔도 좋겠어요!

Comment on lines +33 to +48
private fun setAdjacentTilesClicked(point: Point) {
val adjacent = point.getAdjacentPoints(mineMap.mapInfo.mapSize)
for (adj in adjacent) {
if (clickedSet.contains(adj)) continue

adjacentTileDfs(adj)
}
}

private fun adjacentTileDfs(adj: Point) {
val info = mineMap.mineMap[adj]
if (info is MapTile.Blank) {
setClickedPoint(adj)
if (info.isNoMineNear) setAdjacentTilesClicked(adj)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

수행하고자 하는 것이 무엇인지를 고민해서 메서드명에 녹여내보면 좋겠어요 :)
정답은 없으니 충분히 고민해보는 시간을 가져보셔도 좋겠어요.

Comment on lines +3 to +4
data class Point(val row: Int, val col: Int) {
fun getAdjacentPoints(mapSize: MapSize): List<Point> {
Copy link
Member

Choose a reason for hiding this comment

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

Point객체 자체는 지뢰판의 영향 없이 그대로 주위 point 객체를 모두 반환하는 것이 더 자연스러워 보여요.
지뢰판에서 유효하지 않은 Point를 빼내는 작업은 다른 객체의 역할로 분리해보는 건 어떨까요?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants