-
Notifications
You must be signed in to change notification settings - Fork 206
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
[STEP3] 지뢰 찾기(게임 실행) #329
base: hjjae2
Are you sure you want to change the base?
[STEP3] 지뢰 찾기(게임 실행) #329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 봤습니다!
리뷰 확인 부탁드립니다.
private val width: Int | ||
get() = cells.first().size | ||
|
||
init { | ||
validateHeightIsPositive() | ||
validateWidthIsPositive() | ||
validateSameWidth() | ||
|
||
allNormalCount = (height * width) - minePositions.size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
계산식을 쓰는 것은 성능에 좋겠지만, 값의 양이 적을 때는 조금 더 명확한 코드로 카운트를 세고 계산하는게 버그가 더 적은것 같아요. openAndIncreaseOpenedCount
에서의 방어 코드도 없어질 수 있겠군요~
allNormalCount = cells.sumOf { it.countOfMormals() }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opendCount
도 같은 의견입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요건 주신 리뷰(#329 (comment)) 덕분에 지울 수 있게 됐네요. 👍
fun isWin(): Boolean { | ||
return status == Status.WIN | ||
} | ||
|
||
fun isLose(): Boolean { | ||
return status == Status.LOSE | ||
} | ||
|
||
fun isGameOver(): Boolean { | ||
return isWin() || isLose() | ||
} | ||
|
||
fun isNotGameOver(): Boolean { | ||
return !isGameOver() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드 호출 순서대로 코드의 순서도 변경하면 더 좋겠네요!
- isNotGameOver()
- isGameOver()
- isWin()
- isLose()
enum class Status { | ||
WIN, LOSE, PLAYING, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런식으로도 가능하겠네요~ 그러면 isGameOver()
코드도 조금 바뀔 수 있겠네요!
enum class Status { | |
WIN, LOSE, PLAYING, | |
} | |
enum class Status( | |
val finished: Boolean = false | |
) { | |
WIN(true), LOSE(true), PLAYING, | |
} |
var isOpened: Boolean = false | ||
private set | ||
|
||
fun open() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open
에 return
값을 추가하고(OPEND, FAILED) open
으로 구현하면 다형성을 적용할 수 있지 않을까요?
그러면 Board
에서 코드도 조금 간단해질 수 있을것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if문을 사용해서 Normal인지 체크 하고 주변의 Cell 을 여는 로직을 처리하고 있으니, 절차지향 적으로 조금 느껴지고 있었어요!
그래서 일반 Cell 이 열릴 때 주변 Cell 이 열리도록 수정해보는건 어떨까요?
DM 으로 주신 의견과 함께 조금 고민해봤는데요. 의도하신 게 적용됐을지 헷갈리네요. 😅
Cell 이 열릴 때 주변 Cell 이 열리는 로직을 적용하기 위해 Cells 의 구조를 변경(List<Cell>
-> List<List<Cells>>
)했어요.
↑ 요 로직을 Cells 클래스에 맡기도록 하면 어색함 없이 자연스러운 것 같아서요.
덕분에 Board 쪽엔 Cell 에 대한 로직 보다 게임(?)에 대한 로직(open, win, lose 등)만 남게 된 점도 개선되었다고 생각이 드네요.
} | ||
|
||
private fun allOpened(): Boolean { | ||
return openedCount == allNormalCount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아마 이 부분 때문에 allNormalCount
, openedCount
2개의 변수가 만들어 졌고, 관리가 필요한거네요!
이런식으로 처리해도 괜찮을것 같네요. 다른 개발자가 특별한 알고리즘을 배우지 않아도 충분히 이해가 가능할거에요.
return cells.all { it.allOpended() }
안녕하세요, 리뷰어님!
STEP3 지뢰 찾기(게임 실행) 미션 제출합니다.
편히 리뷰 부탁드립니다! 🙇