[🚀 3단계 - 지뢰 찾기(게임 실행)] 신종화 미션 제출합니다. #487

jjongwa · 2024-12-22T14:56:20Z

안녕하세요~! 지뢰 찾기 3단계 제출합니다!

마찬가지로 지난 단계에 대한 피드백 반영 & 3단계 요구사항 구현했습니다!

게임판 둘레를 추가로 만드는 방식과 좌표를 미리 생성 후 섞어 지뢰 위치를 정하는 방식은 미처 생각하지 못했었는데
확실히 로직의 복잡도가 줄어들었네요 🚀

마지막까지 잘 부탁드려요 🙇

malibinYun

3단계 미션 고생 많으셨어요!
여러 생각해볼만한 곳에 코멘트 남겨두었어요.
피드백 반영 후 다시 리뷰 요청 부탁드려요!

malibinYun · 2024-12-23T16:31:59Z

src/main/kotlin/minesweeper/domain/Field.kt

+    private fun createField(): Map<Position, Spot> {
+        val spots = mutableMapOf<Position, Spot>()
+        for (x in 0 until fieldInfo.getWidth() + 1) {
+            for (y in 0 until fieldInfo.getHeight() + 1) {
+                val position = Position(x, y)
+                spots[position] =
+                    if (minePositions.contains(position)) {
+                        MineSpot(position)
+                    } else {
+                        SafeSpot(position, getNearByMineCount(minePositions, position))
+                    }


지뢰찾기를 처음 시작할 때부터 주변 지뢰의 개수를 넣어줄 수도 있겠지만,
어떤 칸을 열었을 때 계산하도록 구현해보는 건 어떨까요?
아직 열지 않은 칸들을 모두 계산해서 미리 값을 넣어두는 건 낭비일 수도 있겠다는 생각이 있어요 :)

필드 크기가 클수록 불필요하겠네요.!
open 시점에 주변 지뢰 개수를 계산해 업데이트 하도록 변경해 주었습니다! 19f1404

malibinYun · 2024-12-23T16:34:14Z

src/main/kotlin/minesweeper/domain/Field.kt

+        return NearbyDirection.entries
+            .toTypedArray()
+            .count { direction ->
+                val nearbyPosition =
+                    Position(
+                        position.x + direction.dx(),
+                        position.y + direction.dy(),
+                    )
+                minePositions.contains(nearbyPosition)
+            }


Position 객체에게 해당 객체 주변의 Position은 무엇이 있는지 메시지를 던져 반환하도록 구성해보는 건 어떨까요?

val nearbyPositions = position.nearbyPositions()

data class Position(val x: Int, val y: Int) { fun nearbyPositions(): Set<Position> { return NearbyDirection.entries.map { direction -> Position( x + direction.dx(), y + direction.dy(), ) }.toSet() } }

Position이 해당 책임을 가지도록 이동해 주었습니다.
getNearByMineCount의 구조가 훨씬 깔끔해졌네요! 👍

private fun getNearByMineCount( minePositions: Set<Position>, position: Position, ): Int { val nearbyPositions = position.nearbyPositions() return nearbyPositions.count { it in minePositions } }

malibinYun · 2024-12-23T16:38:18Z

src/main/kotlin/minesweeper/domain/Field.kt

+    fun openSpot(position: Position) {
+        val targetSpot = spots[position] as SafeSpot
+        targetSpot.open()


여는 칸의 위치가 지뢰인 경우는 타입 캐스팅 Exception이 나타날 것으로 우려돼요.
지뢰인 경우에 게임이 비정상 종료되지 않도록 처리해보는 건 어떨까요?

기존엔 여는 칸의 위치가 Mine인지 여부를 Game에서 따로 확인한 뒤 openSpot을 호출하는 구조였지만,
Mine인지 여부를 openSpot에 포함시키로 해당 여부를 Boolean값으로 받아 게임 종료를 처리하는 구조로 변경했습니다! 49f3806

malibinYun · 2024-12-23T16:43:16Z

src/main/kotlin/minesweeper/domain/Spot.kt

+    fun open() {
+        require(!isOpened) { "이미 열린 칸입니다." }


이미 열린 칸을 입력하는 경우 게임이 비정상 종료되는 것은 어색하다고 생각해요.
open을 호출하는 곳에서 try-catch로 잡아볼 수도 있겠지만, sealed를 활용해서 결과를 반환해보는 건 어떨까요?
open했을 때, 비정상적이지만 충분히 게임 도메인의 로직인 경우 그에 대한 객체를 생성해서 반환하고, 반환 받은 곳에서는 그 객체의 인스턴스를 비교해서 후 처리를 해도 좋겠어요.

sealed interface OpenResult { object Success: OpenResult object AlreadyOpendSpot: OpenResult } ...

openSpot에서 OpenResult를 반환하도록 변경하고, 이를 통해 Game의 종료 여부를 결정하도록 변경해 주었습니다!
AlreadyOpen의 경우, 이미 열린 칸이라는 메시지를 출력하고 계속 진행할 수 있게 로직도 변경해 주었습니다.

646f2f3

malibinYun · 2024-12-23T16:44:30Z

src/main/kotlin/minesweeper/dto/FieldResponse.kt


 class FieldResponse(private val field: Field) {
-    fun toFormattedStringInitialField(): String {
-        return field.lines.joinToString("\n") { line ->
-            line.spots.joinToString(" ") { spot ->
-                if (spot.isMine()) {
-                    "*"
-                } else {
-                    getNearbyMineCount(spot as SafeSpot)
-                }.toString()
+    fun toFormattedStringField(): String {
+        val fieldInfo = field.getFieldInfo()
+        val positions = generatePositions(fieldInfo.getWidth(), fieldInfo.getHeight())
+
+        return formatField(positions)
+    }
+
+    private fun generatePositions(
+        width: Int,
+        height: Int,
+    ): List<Position> {
+        return (1..width).flatMap { x ->
+            (1..height).map { y ->
+                Position(x, y)
            }
        }
    }

-    private fun getNearbyMineCount(spot: SafeSpot): String {
-        return spot.nearbyMineCount.toString()
+    private fun formatField(positions: List<Position>): String {
+        return positions.groupBy { it.y }
+            .map { (_, positionsInRow) ->
+                positionsInRow.joinToString(" ") { position ->
+                    formatSpot(field.getSpot(position))
+                }
+            }.joinToString("\n")
+    }
+
+    private fun formatSpot(spot: Spot): String {
+        if (spot.isOpened()) {
+            return if (spot is SafeSpot) {
+                spot.nearbyMineCount.toString()
+            } else {
+                "*"
+            }
+        }
+        return "C"


응답 객체 자체에서 콘솔 뷰를 그려내는 건 조금 어색하단 생각이 드네요!
값을 전달만 하고, 뷰를 그려내는 곳은 뷰에서 역할을 수행하도록 분리해보셔도 좋겠어요!

👍
저는 조금 다르게 보았는데요,
VIew는 '어디에 보여줄 것인가'이고, Response는 '어떻게 보여줄 것인가'의 책임을 각각 맡고 있다고 생각했어요!

그래서 해당 Veiw에 맞는 출력 형식으로의 변경은 Response의 메서드를 통해 처리하고,
이를 통해 View 레이어까지 도메인이 노출되는 것을 막아 도메인의 캡슐화를 유지하는 장점도 있다고 생각했습니다.!

malibinYun · 2024-12-23T16:45:30Z

src/test/kotlin/minesweeper/domain/FieldTest.kt

+    /*
+                        1 2 3 4  x
+     * 1 0 0         1  C 1 0 0
+     2 2 2 1     ->  2  C 2 2 1
+     1 * 2 *         3  C C C C
+                     y
+     */


테스트 코드를 부연 설명해주는, 시각적 표현을 도와주는 주석 너무 좋네요! 👍👍

malibinYun

피드백 반영 고생 많으셨어요 👍
지뢰찾기 미션을 너무 잘 구현해주셔서 크게 더 피드백 드릴 부분이 없었네요!
다음 미션은 리팩터링 미션이라, 더 구현하고 싶으신 부분이 있으시다면 반영 후 리뷰 요청 부탁드리고, 이만 마무리 하고 싶으시다면 DM이나 코멘트 부탁드려요!

malibinYun · 2024-12-25T13:21:56Z

src/test/kotlin/minesweeper/domain/PositionTest.kt

+        val position = Position(1, 1)
+        val nearbyPositions = position.nearbyPositions()
+
+        nearbyPositions.size shouldBe 8
+        nearbyPositions shouldContain Position(0, 0)
+        nearbyPositions shouldContain Position(0, 1)
+        nearbyPositions shouldContain Position(0, 2)
+        nearbyPositions shouldContain Position(1, 0)
+        nearbyPositions shouldContain Position(1, 2)
+        nearbyPositions shouldContain Position(2, 0)
+        nearbyPositions shouldContain Position(2, 1)
+        nearbyPositions shouldContain Position(2, 2)


기댓값인 좌표를 set으로 담아, set대 set으로 값을 비교해 테스트를 해보아도 좋겠어요!

set과 set 비교도 간단하게 shouldBe로 가능하군요..! 변경했습니다!
b9f4f1b

malibinYun · 2024-12-25T13:23:06Z

src/main/kotlin/minesweeper/domain/Field.kt

+            }
+            openTargetSpot(it as SafeSpot)


as? 로 nullable 캐스팅을 한 뒤, null 처리를 하는 형태로 로직을 구성해봐도 좋겠어요!

fun openSpot(position: Position): OpenResult { val targetSpot = spots[position]?.let { if (it.isMine()) { return OpenResult.GameOver } it } as SafeSpot val openResult = targetSpot.open() targetSpot.calculateNearbyMineCount(minePositions) checkAndOpenNearbySpot(targetSpot) return openResult }

이런 식으로 let을 통해 null이 아닌 spot에서만 나머지 로직을 실행하도록 바꾸어 주었습니다

malibinYun · 2024-12-25T13:32:36Z

src/main/kotlin/minesweeper/domain/Spot.kt

+    var nearbyMineCount: Int = 0
+


외부에서는 이 필드를 재할당 하지 못하도록, setter를 private으로 막아보는 건 어떨까요?
코틀린 프로퍼티에 대한 문법은 공식 문서를 참고해보세요!

nearbyMineCount에 private set 적용했습니다! d9995d5

malibinYun · 2024-12-25T13:44:39Z

src/main/kotlin/minesweeper/domain/Field.kt

+    fun getFieldInfo(): FieldInfo {
+        return fieldInfo


이 함수는 fieldInfo 멤버의 private을 지워 외부에 프로퍼티를 공개하는 형태로 제거해볼 수 있겠어요!

불필요한 getter를 사용하지 않고 필드의 private을 제거하는 방식으로 변경했습니다! d7db7a3

malibinYun · 2024-12-25T13:46:05Z

src/main/kotlin/minesweeper/domain/Field.kt

+                if (!spot.isOpened()) {
+                    openSpot(it)


spot 객체에 isNotOpened() 혹은 isClosed() 함수를 추가해줘도 좋겠다는 생각이 드네요!

확실히 부정문을 쓰는 것보단 isClosed를 만들어 사용하는 편이 훨씬 더 가독성이 좋아 보이네요! 2bb8cf2

jjongwa · 2024-12-25T15:31:41Z

@malibinYun 이왕 하는 미션 최대한 깔끔하게 끝내고 싶어 남겨주신 리뷰에 대한 피드백 반영했습니다!
혹시 추가로 구조적인 리팩토링 아이디어가 있다면 뭐든지 리뷰 주세요! 🙇 🙇 🙇

jjongwa added 7 commits December 22, 2024 11:14

refactor: 인접칸 enum으로 변경 & 대각선 위치 포함

2e6a43c

refactor: Pair() 대신 좌표를 의미하는 클래스 생성

d84a3d5

refactor: Spot sealed class 적용

1721d41

refactor: 지뢰 위치 설정 및 필드 생성 로직 변경

8ce06c5

docs(README.md): 3단계 기능 구현 목록 작성

e92bd17

feat: Spot에 Open 여부 상태 필드 추가

be1c3fb

feat: 특정 좌표 open 기능 추가

b9edd3c

malibinYun requested changes Dec 23, 2024

View reviewed changes

jjongwa added 6 commits December 25, 2024 00:17

refactor: 주변 좌표 구하는 로직을 Position으로 이동

10d0733

refactor: createField() 로직 구조 변경

912d671

refactor: openSpot에서 타입 캐스팅 에러가 발생하지 않도록 변경

49f3806

refactor: 이미 열린 칸을 입력할 경우 재시도 가능하도록 로직 변경

646f2f3

polish: 메서드 네이밍 변경

d956b42

refactor: open 시점에 주변 지뢰 개수 계산하도록 변경

19f1404

malibinYun requested changes Dec 25, 2024

View reviewed changes

jjongwa added 4 commits December 25, 2024 23:57

test: 근처 좌표 확인 테스트를 set끼리 비교하도록 변경

b9f4f1b

refactor: nearbyMineCount에 private set 설정

d9995d5

refactor: fieldInfo private 제거

d7db7a3

refactor: Spot에 isClosed() 메서드 추가

2bb8cf2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[🚀 3단계 - 지뢰 찾기(게임 실행)] 신종화 미션 제출합니다. #487

[🚀 3단계 - 지뢰 찾기(게임 실행)] 신종화 미션 제출합니다. #487

jjongwa commented Dec 22, 2024

malibinYun left a comment

malibinYun Dec 23, 2024

jjongwa Dec 24, 2024

malibinYun Dec 23, 2024

jjongwa Dec 24, 2024

malibinYun Dec 23, 2024

jjongwa Dec 24, 2024 •

edited

Loading

malibinYun Dec 23, 2024

jjongwa Dec 24, 2024

malibinYun Dec 23, 2024

jjongwa Dec 24, 2024

malibinYun Dec 23, 2024

malibinYun left a comment

malibinYun Dec 25, 2024

jjongwa Dec 25, 2024

malibinYun Dec 25, 2024

jjongwa Dec 25, 2024

malibinYun Dec 25, 2024

jjongwa Dec 25, 2024

malibinYun Dec 25, 2024

jjongwa Dec 25, 2024

malibinYun Dec 25, 2024

jjongwa Dec 25, 2024

jjongwa commented Dec 25, 2024

		fun open() {
		require(!isOpened) { "이미 열린 칸입니다." }

[🚀 3단계 - 지뢰 찾기(게임 실행)] 신종화 미션 제출합니다. #487

Are you sure you want to change the base?

[🚀 3단계 - 지뢰 찾기(게임 실행)] 신종화 미션 제출합니다. #487

Conversation

jjongwa commented Dec 22, 2024

malibinYun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjongwa Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malibinYun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjongwa commented Dec 25, 2024

jjongwa Dec 24, 2024 •

edited

Loading