Step2 by jpoh281 · Pull Request #232 · next-step/kotlin-minesweeper

jpoh281 · 2023-01-02T07:20:39Z

안녕하세요.. 제가 뭔가 커밋을 잘못했는지 꼬였어요.

refactor: ResultView의 매직 리터럴 companion object로 이동

커밋부터 봐주시면 될것같아요.

오늘이 마지막 피드백 날짜라고 해서 진행하던 2단계를 부랴부랴 정리하고 PR 날립니다..

# Conflicts: # src/main/kotlin/minesweeper/README.md # src/main/kotlin/minesweeper/controller/GameController.kt # src/main/kotlin/minesweeper/domain/Block.kt # src/main/kotlin/minesweeper/domain/ExceptionReason.kt # src/main/kotlin/minesweeper/domain/MineSweeperBoard.kt # src/main/kotlin/minesweeper/domain/Point.kt # src/main/kotlin/minesweeper/view/ResultView.kt # src/main/kotlin/minesweeper/view/ResultViewImpl.kt # src/test/kotlin/minesweeper/domain/PointTest.kt

ohgillwhan

코드 잘 봤습니다. MimeSweeperBoard 테스트 코드도 부탁드려용!

ohgillwhan · 2023-01-03T14:06:21Z

+        } catch (e: MineSweeperException) {
+            resultView.printKnownException(e)
+            null
+        } catch (e: Exception) {
+            resultView.printUnknownException(e)
+            null
+        }


Exception을 고스란히 터트려주는것도 좋은 방법이에요.

게임을 종료시킬 수 있는것을 컨트롤러에서 제어가 가능해지죠.

ohgillwhan · 2023-01-03T14:08:09Z

@@ -4,14 +4,29 @@ sealed class Block {
    abstract fun open()


열 수 있는 블록과 없는 블록을 나누는건 어떨까요?
TODO("Not yet implemented") 가 나와서 애매해지는거 같아요.

ohgillwhan · 2023-01-03T14:09:20Z

        _state = buildBoard(width, height).toMutableMap()
        plantMines(mineCount)
        _mineCount = countMine()
+        buildSafeBlock()


이 코드는 buildBoard를 할 때 같이 이뤄져야 하지 않나요?

ohgillwhan · 2023-01-03T14:09:41Z

        _state = buildBoard(width, height).toMutableMap()
        plantMines(mineCount)
        _mineCount = countMine()
+        buildSafeBlock()


이 코드가 응집이 되어보는건 어떨까요?

ohgillwhan · 2023-01-03T14:10:55Z

            val width = Random().nextInt(width)
            val height = Random().nextInt(height)


랜덤에 의존적인게 아쉽네요. 전략패턴 등을 이용해보면 어떤가요?

ohgillwhan · 2023-01-03T14:13:04Z

+    private fun plusOrNull(x: Int, y: Int): Point? {
+        return try {
+            Point(this.x + x, this.y + y)
+        } catch (e: Exception) {


try catch로 로직을 수행하는게 조금 아쉽네요. 현재 객체에서 x, y 만큼 더 할 수 있는지 메서드를 만들어서 처리해보는건 어떨까요?

jpoh281 · 2023-01-10T00:26:20Z

안녕하세요?
처음에 열 수 있는 블록과 열 수 없는 블록으로 구분했다가, 응집도를 높이다보니 열 수 없는 블록이 필요가 없어지는 것 같아서 다시 삭제했습니다.
전략패턴을 통해서 나머지 계산의 방식으로?? 지뢰가 설치 되도록 구현했습니다.
try - catch 안쓰고 주변의 수 받아올 수 있도록 변경했어요! 엄청 어려울 것 같다고 생각했었는데 생각보다 어렵지않았네요.

ohgillwhan

간단한 리뷰 몇개 남겨봤어요! 확인 부탁드립니다.

ohgillwhan · 2023-01-11T10:04:01Z

-        return gameBoard.state
+    fun run(): MineSweeperBoard? {
+        return try {
+            val gameBoard = setUp()


타입이 나오면 좋겠네요~

ohgillwhan · 2023-01-11T10:05:13Z

+
    override fun compareTo(other: Point) = compareValuesBy(this, other, { it.y }, { it.x })
+
+    private fun plusOrNull(x: Int, y: Int, maxX: Int, maxY: Int): Point? {


연산자 오버로딩을 해도 괜찮겠네요~

ohgillwhan · 2023-01-11T10:05:49Z

+    override fun open() {
+        TODO("Not yet implemented")
+    }


구현이 안될 메서드를 만들어야 하는건 조금 아쉽네요. Block을 세분화 해보는건 어떨까요?

ohgillwhan · 2023-01-11T10:07:29Z

+
+

린트 돌려주세요~

ohgillwhan · 2023-01-11T10:09:19Z

+        val userInputMineCount = 1
+
+        val controller =
+            GameController(FakeInputView(userInputHeight, userInputWidth, userInputMineCount), ResultViewImpl())


ResultViewImpl 보다 더 좋은 의미는 없을까요? ConsoleResultView 이런식으로요

ohgillwhan · 2023-01-11T10:09:33Z

+
+    @Test
+    fun `0보다 작은 수는 생성될 수 없다`() {
+        val exception = org.junit.jupiter.api.assertThrows<MineSweeperException> {


import를 해볼까요?

ohgillwhan · 2023-01-11T10:10:45Z

+        val minePoints = strategy.createMines(width, height, plantingMineCount)
+        minePoints.forEach {
+            _state[it] = MineBlock()
+        }
+        _mineCount = countMine()
+        updateSafeBlock()


전략을 바꿀 수 있으니, 여러 장점이 있을것 같아요.
테스트코드에서 전략을 한개 만들어서, 그 전략만큼 지뢰가 잘 심겼는지 등 테스트 할 수 있겠죠? 나중에 게임 실행하면서 원하는 위치에 지뢰를 심고, 터트리는 등 테스트를 더욱 쉽게 가능할거에요.

jpoh281 · 2023-01-17T05:36:26Z

안녕하세요?
공백이나, 임포트 같은 부분 신경 못쓰고 PR 드려서 죄송해요..

최근에 개발할 때 인터페이스와 구현체가 거의 일방적인 구현체여서 네이밍을 impl 만 사용했었는데, 이렇게 더 명확하게 바꿀 수 있다는걸 다시 생각해볼 수 있어서 너무 좋았어요.
plusOrNull은 오퍼레이터를 오버라이딩하고 싶었는데 이게 제가 계산하는 방식이 null을 사용해야해서 다시 로직을 생각해보는게 좋을까요?
모든 블록에 isOpen 이라는 값을 두고, MineBlock, SafeBlock 둘 다 open 메소드를 구현해서 사용할까 했었는데 피드백을 반영해서
아직 열어보지 않은 UnopenedBlock과 열린 상태인 OpenedBlock을 상속받은 MineBlock, SafeBlock으로 나누었습니다.
MineSweeperBoard를 step3에서 어떻게 테스트 해야할지 감이 잡혔습니다. 감사해요!

adforus_hjp added 27 commits December 22, 2022 12:28

docs: 요구사항 정리

6ce91cc

feat: 높이와 너비, 지뢰의 개수를 반영해 보드를 생성할 수 있다.

288f98a

refactor: 블록 문자열을 블록 객체로 수정

420c9f2

feat:유저로부터 요구사항을 입력받기 위한 InputView 생성

13926cd

feat: Main 및 컨트롤러를 생성해서 유저의 요구사항을 받아 보드 생성

d9b1a77

feat: ResultView를 생성해서 생성된 보드 유저에게 보여주기

563b905

docs: 피드백 목록 정리

cdbdfb7

refactor: View에서 처리해야할 로직들 이동

de0089e

refactor: OrNull 제거

90f6ef0

feat: 좌표를 나타내는 Point 객체 생성

b196d6a

refactor: 이차원 배열 -> 맵으로 수정

70b8fda

feat: CustomException 사용

257b76b

refactor: View를 Interface로 변경

4133782

refactor: 지뢰의 수를 측정하는 메소드와 변수 정리

e7e1a31

refactor: ResultView의 매직 리터럴 companion object로 이동

ada31aa

refactor: forEach -> flatMap으로 변경

cb645f4

refactor: 에러 문구 ResultView에게 책임 부여 및 Known, Unknown의 처리 분류

77a340b

refactor: 보드 자체를 반환하도록 변경

19bdc2c

docs: 2단계 요구사항 작성

9820e71

feat: SafeBlock에 nearMineCount 프로퍼티 추가

3d033cd

feat: 시작할 때 SAFEBLOCK -> EMPTYBLOCK으로 변경

f9e24c6

feat: InputView의 가짜 객체를 만들어서 컨트롤러 테스트 추가

b9b28d3

feat: Point - 근처의 포인트 객체들 받아오기 기능 추가

eea4988

feat: - 지뢰를 심은 뒤에 지뢰의 수를 반영하는 메소드 생성

15dee1e

feat: - SafeBlock 표기 방식 변경

f5b8a13

refactor: 변수명 수정

e1aace3

ohgillwhan requested changes Jan 3, 2023

View reviewed changes

jpoh281 added 2 commits January 3, 2023 23:28

refactor: Controller에서 그대로 터뜨리기

943e4d5

feat: 지뢰를 심는 전략패턴 인터페이스와 클래스들 생성

0e68951

adforus_hjp added 5 commits January 9, 2023 13:08

refactor: 열 수 있는 블록과 열 수 없는 블록 분리

74acd9a

refactor: 전략패턴 적용 및 응집도 높이기

28edd9d

refactor: Empty Block 다시 삭제

b8babef

refactor: try-catch를 안쓰고 로직을 변경

322b8ca

refactor: block, exception 패키지 생성

001d298

ohgillwhan requested changes Jan 11, 2023

View reviewed changes

adforus_hjp added 6 commits January 16, 2023 12:40

refactor: 타입이 나오도록 변경 및 공백 삭제, 임포트 수정

75508fc

refactor: ResultViewImpl -> ConsoleResultView 변경

8e04e5f

feat: 열린 블록과 열리지 않은 블록 클래스를 만들어 open 메소드를 필요한 클래스에만 할당

520a042

refactor: 지뢰가 0일 때 0 나누기 에러 나오는 현상 수정

1c4ee11

refactor: ResultViewImpl -> ConsoleReulstView로 더 명확하게변경

e95a24c

refactor: 코드 정리

11ac2ce

		val width = Random().nextInt(width)
		val height = Random().nextInt(height)


		override fun compareTo(other: Point) = compareValuesBy(this, other, { it.y }, { it.x })

		private fun plusOrNull(x: Int, y: Int, maxX: Int, maxY: Int): Point? {

Uh oh!

Conversation

jpoh281 commented Jan 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ohgillwhan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpoh281 commented Jan 10, 2023

Uh oh!

ohgillwhan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpoh281 commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jpoh281 commented Jan 2, 2023 •

edited

Loading

jpoh281 commented Jan 17, 2023 •

edited

Loading