[Step2]: 🚀 2단계 - 지뢰 찾기(지뢰 개수) #414

dlwnsgus777 · 2023-12-11T02:38:37Z

안녕하세요 리뷰어님!!
강의 종료 후에도 리뷰해주신다니 그저 감사할 따름입니다 🙏

이번 2단계 리뷰도 잘 부탁드립니다!!!

namjackson

안녕하세요 준현님!
2단계 고생하셨습니다,
몇가지 코멘트 남겼으니, 확인해주세요 :)

namjackson · 2023-12-12T09:38:42Z

src/main/kotlin/mineswipper/domain/map/Field.kt

+        if (minePositions.contains(position)) return Mine()
+        return NormalPedal()


Suggested change

if (minePositions.contains(position)) return Mine()

return NormalPedal()

if (minePositions.contains(position)) Mine() else NormalPedal()

더 간단히 사용해보아요!

namjackson · 2023-12-12T09:39:14Z

src/main/kotlin/mineswipper/domain/map/Field.kt

+    }
+
+    companion object {
+        private const val VALID_MESSAGE: String = "오류가 발생했습니다."


구체적으로 어떤 에러인지 표기하면 어떨까요?

namjackson · 2023-12-12T09:40:34Z

src/main/kotlin/mineswipper/domain/map/position/Position.kt

+        return (x - 1..x + 1).flatMap { xVal ->
+            (y - 1..y + 1).map { yVal -> Position(xVal, yVal) }


x - 1, x - 1, y+1,y-1 등이 무엇을 의미할까요?
따로 객체를 관리해보면 어떨까요?

namjackson · 2023-12-12T09:53:50Z

src/main/kotlin/mineswipper/domain/map/Mine.kt

@@ -1,3 +1,6 @@
 package mineswipper.domain.map

-class Mine : Pedal
+class Mine(
+    override var mark: Mark? = Mark("*")


var 로 정의한 이유가 잇을까요?
외부에서 쉽게 변경이 가능한 구조네요!

namjackson · 2023-12-12T09:54:54Z

src/main/kotlin/mineswipper/domain/map/Pedal.kt

nullable 변수로 정의 한 이유가 있을까요?
또 불변객체로 만들어보는건 어떨까요?

namjackson · 2023-12-12T09:56:20Z

src/main/kotlin/mineswipper/domain/map/position/Position.kt

+
+}
+
+fun List<Position>.toPositions(): Positions = Positions(this)


설정 옵션을 활성화하여 EOF 경고를 없애주시면 좋을거같아요!
(File > Preferences > Editor > General > Ensure every saved file ends with a line break )

namjackson · 2023-12-12T09:57:45Z

src/main/kotlin/mineswipper/domain/map/Field.kt

+        val pedals = field[position.toRow()]
+        require(pedals != null) { VALID_MESSAGE }
+
+        val pedal = pedals.get(position.x)


position.x 에 해당하는 값이 없으면 어떤일이 일어날까요/

namjackson · 2023-12-13T14:21:03Z

src/main/kotlin/mineswipper/MineGame.kt

@@ -22,6 +23,8 @@ class MineGame(
            positionFactory.generateMinePositions(size, mine)
        )

+        MarkGenerator.markGeneration(field)


파라미터로 전달된 field가 MarkGenerator내부에서 변경이 되네요!
markGeneration를 통해 field가 변경될수 있다는 점을 생각할수 있을까요?
고민해보면 좋을거같아요!

namjackson · 2023-12-13T14:22:06Z

src/main/kotlin/mineswipper/MineGame.kt

@@ -22,6 +23,8 @@ class MineGame(
            positionFactory.generateMinePositions(size, mine)
        )

+        MarkGenerator.markGeneration(field)
+
        outputManager.printStartGame()
        outputManager.printField(field)
    }


MineGame은 사실상 Controller 역할을 해주고 있어요,
Controller은 View와의 상호작용의 책임을 가졌다고 생각이 들어요,
그렇다면 MineGame내에서, 지뢰를 생성하고, mark하는 비즈니스로직을 분리해보면 어떨까요?

Ted1_Lee and others added 14 commits December 8, 2023 09:00

refactor: 리턴 타입 명시

33b75e5

refactor: 사용하지 않는 함수 제거

4384cfe

feat: 파라미터 이름 변경

937bfd1

feat: 파라미터 이름 변경

87e088f

feat: 리뷰 반영

d7f3900

feat: 필드의 row를 의미하는 Row 객체 생성

f7c94a6

feat: int를 감싼 Row 클래스 생성

3f540f9

feat: 지뢰나 일반 발판의 값을 의미하는 Mark 클래스 추가

8bfbda7

feat: position 패키지 추가

e70df0b

feat: positions 일급 컬렉션 추가

f7eebaf

feat: 지뢰 갯수 출력 기능 개발

57ec133

feat: 테스트 케이스 추가

12754be

feat: 지뢰 갯수 설정 클래스 분리

875d734

feat: 에러 메시지 추가

140ec03

namjackson requested changes Dec 13, 2023

View reviewed changes

Ted1_Lee and others added 8 commits December 14, 2023 09:00

feat: 함수를 더 간단하게 변경

31e02a4

feat: 에러 메시지 변경

fdbda58

feat: editor 설정 변경

cfdce4b

feat: 에러 검증 추가

0a4a2ae

feat: 컨트롤러 로직 줄이기

632971f

feat: ktlint

227cf4c

feat: FieldFactory 생성

90a2b65

feat: 값이 없을 때 처리 추가

7e62807

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Step2]: 🚀 2단계 - 지뢰 찾기(지뢰 개수) #414

[Step2]: 🚀 2단계 - 지뢰 찾기(지뢰 개수) #414

dlwnsgus777 commented Dec 11, 2023

namjackson left a comment

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 12, 2023

namjackson Dec 13, 2023

namjackson Dec 13, 2023

		if (minePositions.contains(position)) return Mine()
		return NormalPedal()

	if (minePositions.contains(position)) return Mine()
	return NormalPedal()
	if (minePositions.contains(position)) Mine() else NormalPedal()

		return (x - 1..x + 1).flatMap { xVal ->
		(y - 1..y + 1).map { yVal -> Position(xVal, yVal) }


		}

		fun List<Position>.toPositions(): Positions = Positions(this)

[Step2]: 🚀 2단계 - 지뢰 찾기(지뢰 개수) #414

Are you sure you want to change the base?

[Step2]: 🚀 2단계 - 지뢰 찾기(지뢰 개수) #414

Conversation

dlwnsgus777 commented Dec 11, 2023

namjackson 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