-
Notifications
You must be signed in to change notification settings - Fork 180
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
[Step1] 지뢰 찾기(그리기) #346
base: songyi00
Are you sure you want to change the base?
[Step1] 지뢰 찾기(그리기) #346
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.
안녕하세요!!
늦었지만 지뢰찾기 미션 코멘트드립니당
끝까지 화이팅입니다!! 👍
@@ -0,0 +1,20 @@ | |||
# 지뢰 찾기 |
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.
기능 요구사항 작성 👍
src/main/kotlin/view/InputView.kt
Outdated
|
||
private fun requestHeight(): Int { | ||
println("높이를 입력하세요.") | ||
return readln().toInt() |
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.
toIntOrNull()
을 활용해서 숫자가 아닌 값에 대한 에러처리를 핸들링해주면 어떨까요?
package domain | ||
|
||
data class BoardSize( | ||
val width: Int, |
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.
로또미션에서 했던 것 처럼 Value Object를 만들어서 간단한 validation을 수행해보면 어떨까요?
val board = initBoard() | ||
|
||
for (i in 0 until boardSize.height) { | ||
for (j in 0 until boardSize.width) { |
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.
indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
위 요구사항을 만족하도록 리팩토링해보면 어떨까요?
return BoardInfo(board.map { it.toList() }) | ||
} | ||
|
||
private fun initBoard(): Array<Array<Cell>> { |
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.
이중 Array보다는 조금 더 작은 단위의 객체를 만들어보면 어떨까요?
열(Column)과 행(Row) class들이 나올 수 있을 것 같아요 :)
} | ||
|
||
private fun randomPoint(boardSize: BoardSize): Point { | ||
val randomY = (1 until boardSize.height).random() |
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.
0부터가 아니라 1부터인 이유가 있을까요?
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.
@vsh123 오타인 것 같네요😅
package domain | ||
|
||
data class MineLocations( | ||
val points: List<Point> |
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.
List보다는 Set이 조금 더 낫지 않을까요?
두 컬렉션타입에 따른 contains구현방식에 대해서도 알아보시면 좋을 것 같아요 :)
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.
그렇네요 굳이 리스트가 필요한 상황은 아니네요!! Set을 사용할 때 검색 속도가 더 빠르겠네요 👍
val mineCount: Int, | ||
val boardInfoGenerator: BoardInfoGenerator = BoardInfoGenerator(boardSize, mineCount) | ||
) { | ||
val info: BoardInfo by lazy { |
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.
지연로딩을 사용하신 이유가 궁금합니다~!!
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.
@vsh123 한번 만들어진 지뢰판의 info
를 호출할 때마다 내부 값을 생성하는 로직을 호출하기보다는 MineBoard
자체를 불변객체로 만들고 한번 만들어진 지뢰판의 경우 내부값을 한번만 초기화하도록 하려고 했습니다 :)
src/main/kotlin/domain/Cell.kt
Outdated
package domain | ||
|
||
data class Cell( | ||
val value: Char |
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.
해당 문자 값은 View의 영역으로 보여요!!
만약 PC에서는 지뢰를 *로 표시하지만, Mobiled에서는 x로 표현한다면 같은 도메인 로직을 사용하지 못할 것 같아서요
조금 더 유연한 구조를 고민해보면 어떨까요?
안녕하세요 리뷰어님 😁
늦었지만 꼭 완주하고 싶습니다!!
리뷰 잘 부탁립니다!! 🙇🏻♀️