Step1 - 지뢰 찾기(그리기) #333
Open
Step1 - 지뢰 찾기(그리기) #333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BeokBeok
requested changes
Jul 9, 2023
| @@ -0,0 +1,33 @@ | |||
| package minesweeper.domain | |||
|
|
|||
| class MineMap(height: Height, val width: Width, mineCount: MineCount) { | |||
There was a problem hiding this comment.
한 가지 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다. 이 규칙을 MineMap 클래스에 적용하려고 했는데, 좋은 방법이 떠오르지 않아서.. 3개의 인스턴스를 받게 유지했습니다. 좋은 방법이 있다면 피드백 부탁드리겠습니다!
Width와 Height를 묶을 수 있지 않을까요? 🤔
Comment on lines
15
to
21
| (1..height.value).forEach { y -> | ||
| (1..width.value).forEach { x -> | ||
| val index = (y - 1) * width.value + (x - 1) | ||
|
|
||
| mutableList.add(point(mineIndexes.contains(index), x, y)) | ||
| } | ||
| } |
There was a problem hiding this comment.
객체지향 생활 체조 원칙에는 아래와 같은 항목이 있습니다.
한 메서드에 오직 한 단계의 들여쓰기만 한다.
이 요구사항을 적용해보면 어떨까요?
| return input | ||
| } | ||
|
|
||
| private fun receiveInt(): Int { |
There was a problem hiding this comment.
함수명만 봤을 때에는 "Int를 받는다" 라고 이해가 되는데요.
여기서 Int는 어떤 것을 받는 걸까요?
함수명을 좀 더 구체적으로 작성해보면 어떨까요?
| } | ||
|
|
||
| private fun receiveInt(): Int { | ||
| var int: Int? = receiveString().toIntOrNull() |
There was a problem hiding this comment.
변수는 상태를 나타냅니다.
"Int"와 같은 자료형은 상태를 나타내기 어렵습니다.
변수명만 봤을 때 어떤 상태를 가지는지 알 수 있도록 이름을 변경해보면 어떨까요?
|
|
||
| class MineMapTest { | ||
| @Test | ||
| fun `지뢰 개수가 지도 크기를 초과하면 예외가 발생한다`() { |
There was a problem hiding this comment.
내가 원하는 좌표에 지뢰를 심어보고, 그 좌표에 정말 지뢰가 있는지 확인해볼 수 있도록 테스트 코드를 작성해보면 어떨까요?
- 3개 이상의 인스턴스 변수를 가지는 클래스 제거 - 한 단계의 들여쓰기만 적용
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 리뷰어님 지뢰 찾기 미션 리뷰 잘 부탁드리겠습니다!
한 가지
3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.이 규칙을 MineMap 클래스에 적용하려고 했는데, 좋은 방법이 떠오르지 않아서.. 3개의 인스턴스를 받게 유지했습니다. 좋은 방법이 있다면 피드백 부탁드리겠습니다!