Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

🚀 2단계 - 지뢰 찾기(지뢰 개수) #345

Open
wants to merge 13 commits into
base: jonghyunhub
Choose a base branch
from

Conversation

jonghyunhub
Copy link

안녕하세요!
말씀해주신 불변을 지키는 것을 목표로 코드를 만들어봤습니다~
잘 부탁드리겠습니다 🙏🏻

Copy link

@ohgillwhan ohgillwhan left a comment

Choose a reason for hiding this comment

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

코드 잘 봤습니다!
리뷰 확인 부탁드리며, DM은 언제든 환영입니다!

Comment on lines +16 to +25
class MineSquare() : GameBoardSquare(0) {
override fun isMine(): Boolean = true
}

override fun hashCode(): Int {
return squareValueType.hashCode()
class NumberSquare(numOfNearByMine: Int) : GameBoardSquare(numOfNearByMine) {
override fun isMine(): Boolean = false

companion object {
fun createEmpty(): NumberSquare = NumberSquare(numOfNearByMine = 0)
}

Choose a reason for hiding this comment

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

sealed class 바깥으로 빼도 될거 같아요

require(squareValueType == SquareValueType.EMPTY) { "게임판을 초기화 할 때는 지뢰가 되면 안됩니다." }
}
sealed class GameBoardSquare(val numOfNearByMine: Int) {
abstract fun isMine(): Boolean

Choose a reason for hiding this comment

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

GameBoardSquareMine 타입까지 이미 고려하고 있는것 같네요!
그것 때문에 NumberSquareMineSquare과 논리적으로 엮일려는거 같아요.

Comment on lines 14 to 15
require(x in (startIndexOfBoard..board.first().size)) { "생성된 지뢰의 x좌표는 게임판 범위에 포함되어야 합니다." }
require(y in (startIndexOfBoard..board.size)) { "생성된 지뢰의 y좌표는 게임판 범위에 포함되어야 합니다." }

Choose a reason for hiding this comment

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

startIndexOfBoardboard.first().size, board.size는 무슨 의미를 가지고 있는지 알기 어렵네요!

}

private fun isLocationInIndexOfBoard(board: List<List<GameBoardSquare>>, mineLocation: MineLocation) {
val (x, y) = mineLocation.location
private fun isLocationInIndexOfBoard(board: List<List<GameBoardSquare>>, squareLocation: SquareLocation) {

Choose a reason for hiding this comment

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

is로 시작하니, 리턴이 boolean 일것 같네요! require같은 이름은 어떨까요?

val generatedMineLocation = mineLocationGenerator.generateMineLocation(getBoard())
insertMine(generatedMineLocation)
val (x, y) = mineLocationGenerator.generateMineLocation(mutableBoard)
mutableBoard[y][x] = GameBoardSquare.MineSquare()

Choose a reason for hiding this comment

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

이 부분에서 이미 지뢰를 생성하고 있네요! 그럼 이 클래스 내에서 지뢰(MineSquare)를 담고 있는 List를 만들어서 관리해주면 GameBoardSquareisMine은 고민해볼 필요가 있겠어요!

@@ -0,0 +1,54 @@
package minesweeper.domain

object MineCounter {

Choose a reason for hiding this comment

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

fun 구현이 2depth가 안되도록 해볼까요?


object MineCounter {

fun initMineCounts(mutableBoard: MutableList<MutableList<GameBoardSquare>>): List<List<GameBoardSquare>> {

Choose a reason for hiding this comment

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

타입은 최대한 mutable을 피해볼까요?

Comment on lines +45 to +53
private fun isNewLocationInBoardBound(
newLocation: SquareLocation,
board: List<List<GameBoardSquare>>
) = newLocation.x in board.indices && newLocation.y in board[newLocation.x].indices

private fun isNewLocationIsMine(
board: List<List<GameBoardSquare>>,
newLocation: SquareLocation
) = board[newLocation.x][newLocation.y].isMine()

Choose a reason for hiding this comment

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

= 으로 fun을 바로 정의할 떄는 리턴 타입을 명시해봐요!

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

Successfully merging this pull request may close these issues.

2 participants