🚀 1단계 - 지뢰 찾기(그리기) #417

giibeom · 2023-12-14T03:02:13Z

안녕하세요 리뷰어님 :)

완주를 위해 열심히 달리고 있습니다 ㅎㅎ 🚀
잘 부탁드립니다 🙏🙏

giibeom

뭔가 이번 step1에서는 단위테스트를 진행할만한 도메인 로직이 보이지 않았는데요 🤔🤔
(굳이 한다면... MineGenerator#create 정도..?)
뭔가 TDD 과정이다 보니까 테스트 코드 없이 PR 드리는게 좀 찜찜...해서 코멘트 달아봅니다 😂

giibeom · 2023-12-14T03:10:29Z

src/main/kotlin/game/minesweeper/ui/ConsoleView.kt

+        val height = readlnOrNull()?.toIntOrNull()
+        requireNotNull(height) { INVALID_NUMBER_PROMPT }
+        require(height > 0) { INVALID_HEIGHT_RANGE_PROMPT }


readln()을 사용할까 하다가IllegalArgumentException로 통일하는게 좋을 것 같아서 위와 같이 코드를 짜보았습니당 🤔

namjackson

안녕하세요 기범님!
연말이라 정신이 없어서, 리뷰가 늦었네요ㅠㅠ!
몇가지 코멘트 남겼으니, 확인해주세요 :)

namjackson · 2023-12-21T08:41:57Z

src/main/kotlin/game/minesweeper/domain/GameBoard.kt

+package game.minesweeper.domain
+
+class GameBoard(height: Int, width: Int) {
+    private val board = Array(height) { Array(width) { "C" } }


"C"라는건 무엇을 의미할까요?

모든 원시 값과 문자열을 포장한다.

요구사항을 지켜보아요!

namjackson · 2023-12-21T08:43:23Z

src/main/kotlin/game/minesweeper/domain/GameBoard.kt

+        return board.joinToString("\n") { row ->
+            row.joinToString(" ")
+        }


해당 로직은 View관련 로직은 아닐까요?
View가 텍스트로 진행되지 않는다면 domain로직에도 영향이 가는구조네요!

namjackson · 2023-12-21T08:44:17Z

src/main/kotlin/game/minesweeper/domain/MineGenerator.kt

+package game.minesweeper.domain
+
+object MineGenerator {
+    fun create(board: Array<Array<String>>, minesNumber: Int) {


Array<Array>는 무엇을 의미할까요?

일급 컬렉션을 쓴다.

을 활용해보면 어떨까요?

namjackson · 2023-12-21T08:45:00Z

src/main/kotlin/game/minesweeper/domain/MineGenerator.kt

+            if (board[randomRow][randomCol] != "*") {
+                board[randomRow][randomCol] = "*"
+                minesPlaced++
+            }


한 메서드에 오직 한 단계의 들여쓰기만 한다.

요구사항을 지켜보아요!

namjackson · 2023-12-21T08:45:20Z

src/main/kotlin/game/minesweeper/domain/MineGenerator.kt

+            val randomCol = (0 until width).random()
+
+            if (board[randomRow][randomCol] != "*") {
+                board[randomRow][randomCol] = "*"


"*"은 무엇을 의미할까요? 마찬가지로 객체로 관리해보면 어떨까요?

namjackson · 2023-12-21T08:46:12Z

src/main/kotlin/game/minesweeper/domain/MineGenerator.kt

+            val randomCol = (0 until width).random()
+
+            if (board[randomRow][randomCol] != "*") {
+                board[randomRow][randomCol] = "*"


파리미터로 전달받은 board가 변경되는 구조네요,
외부에서 board가 변경되었다는걸 알수있을까요?
추가로 고민해보아요!

namjackson · 2023-12-21T08:48:50Z

src/main/kotlin/game/minesweeper/domain/GameBoard.kt

+    private val board = Array(height) { Array(width) { "C" } }
+
+    fun startGame(minesNumber: Int) {
+        MineGenerator.create(board, minesNumber)


MineGenerator라는 싱글턴객체에 접근하는 구조네요
싱글턴패턴은 객체간 결합도를 올리는 경우도 많아서 사용에 주의하는게 좋아요!

namjackson · 2023-12-21T08:50:42Z

src/test/kotlin/game/minesweeper/domain/MineGeneratorTest.kt

+import io.kotest.data.row
+import io.kotest.matchers.shouldBe
+
+class MineGeneratorTest : StringSpec({


GameBoard 관련 테스트 코드도 작성해보면 어떨까요?

giibeom added 3 commits December 14, 2023 02:01

docs: 기능 요구사항을 분석하고 정리한다

4b9794d

feat: 높이와 너비, 지뢰 개수를 입력받는다

cb274d8

feat: 지뢰를 생성하고, 게임 보드를 출력한다

2a0cb93

giibeom commented Dec 14, 2023

View reviewed changes

test: 지뢰 생성 기능을 테스트한다

19f43ca

namjackson requested changes Dec 21, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 1단계 - 지뢰 찾기(그리기) #417

🚀 1단계 - 지뢰 찾기(그리기) #417

giibeom commented Dec 14, 2023

giibeom left a comment •

edited

giibeom Dec 14, 2023

namjackson left a comment

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

namjackson Dec 21, 2023

🚀 1단계 - 지뢰 찾기(그리기) #417

Are you sure you want to change the base?

🚀 1단계 - 지뢰 찾기(그리기) #417

Conversation

giibeom commented Dec 14, 2023

giibeom left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

giibeom left a comment •

edited