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

[스캇] 3, 4단계 오목 제출합니다. #42

Merged
merged 28 commits into from
Mar 27, 2023

Conversation

chws0508
Copy link

안녕하세요, 스캇입니다!
최대한 도메인을 수정하지 않고 코드를 작성해보려고 했습니다.
이번 리뷰 하면서 수정했던점, 그리고 궁금했던점 몇가지 남겨봅니다!

  1. 안드로이드에 적용하기 전에는 도메인에있는 OmokGame에서 뷰 영역의 함수를 고차함수로 넘겨받는 함수를 가지고 있었는데, 안드로이드를 적용하려고 하니까, 코드 실행 순서가 콘솔과 안드로이드 둘 모두를 만족시키기 어려워 고차함수 인자를 빼주었습니다. 그리고, 도메인에서 while 문을 돌려 승자를 찾는 방식이었지만, 안드로이드를 적용시키려고 하니 이게 불가능하여 while 문을 빼주었습니다.
  2. 그래서 결국 while 문을 Controller에 넣을 수 밖에 없었는데(콘솔기준), 이는 Controller 가 도메인 로직을 갖게되어 MVC에 적합하지 않은 것일까요?
  3. 그리고 nullable 타입을 이용해 분기처리하는 로직을 많이 사용하였습니다. ex) 승자가 생기면 승자반환, 아니면 null 반환, 돌을 둘 수있으면 stone을 반환, 아니면 null을 반환
    이로 인한 문제점은 따로 없을까요?

항상 좋은 리뷰해주셔서 감사합니다!

Copy link

@laco-dev laco-dev left a comment

Choose a reason for hiding this comment

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

안드로이드 미션 고생 많으셨습니다.

도메인을 비롯해 몇가지 고민해볼만한 내용을 남겼으니 충분히 고민 후 도전해보세요.

domain/src/main/kotlin/domain/OmokGame.kt Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/Stones.kt Show resolved Hide resolved
app/src/main/java/woowacourse/omok/MainActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/woowacourse/omok/MainActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/woowacourse/omok/OmokDbManager.kt Outdated Show resolved Hide resolved
app/src/main/java/woowacourse/omok/OmokDbManager.kt Outdated Show resolved Hide resolved
@laco-dev
Copy link

안드로이드에 적용하기 전에는 도메인에있는 OmokGame에서 뷰 영역의 함수를 고차함수로 넘겨받는 함수를 가지고 있었는데, 안드로이드를 적용하려고 하니까, 코드 실행 순서가 콘솔과 안드로이드 둘 모두를 만족시키기 어려워 고차함수 인자를 빼주었습니다. 그리고, 도메인에서 while 문을 돌려 승자를 찾는 방식이었지만, 안드로이드를 적용시키려고 하니 이게 불가능하여 while 문을 빼주었습니다.

자연스러운 방법입니다.
오목 게임이 흐름에 따라 전체적인 게임 과정을 제어할 수도 있지만 게임을 구성하면서 인터페이스를 통해 상태를 변경하는 것도 방법입니다.

물론 마스터 오목 게임 객체를 만들 수 있지만 동기와 비동기 동작 등 많이 어려운 개념이 필요하기 때문에 고민 하신 점으로 충분합니다.

@laco-dev
Copy link

그래서 결국 while 문을 Controller에 넣을 수 밖에 없었는데(콘솔기준), 이는 Controller 가 도메인 로직을 갖게되어 MVC에 적합하지 않은 것일까요?

앞선 의견처럼 도메인을 어떤 객체로 해석하는가에 따라 패턴과 적합한가 아닌가를 생각 해 볼 수 있습니다.
안드로이드의 MVC 패턴에서는 일반적으로 액티비티가 컨트롤러의 역할을 수행합니다.

@laco-dev
Copy link

그리고 nullable 타입을 이용해 분기처리하는 로직을 많이 사용하였습니다. ex) 승자가 생기면 승자반환, 아니면 null 반환, 돌을 둘 수있으면 stone을 반환, 아니면 null을 반환
이로 인한 문제점은 따로 없을까요?

논리적으로 이해가 되는 결과라면 null을 반환하는데 문제가 없습니다.
대표적으로 find 함수같은 경우가 이에 해당하겠습니다.

그렇지만 실패를 비롯해 왜 null을 반환하는것인지 예측할 수 없다면 좋은 방법이 아닙니다.
이 경우 반환 타입에 대한 객체를 만들어 명시적으로 반환하도록 하는 방법이 있습니다.

Copy link

@laco-dev laco-dev left a comment

Choose a reason for hiding this comment

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

피드백 반영 고생하셨습니다.

마지막 미션인 만큼, 개선 해보면 좋을만한 내용을 몇가지 남겨드렸습니다.

domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/Stones.kt Outdated Show resolved Hide resolved
domain/src/main/kotlin/domain/OmokGame.kt Show resolved Hide resolved
Comment on lines 83 to 104
private fun clickBoard(index: Int, view: ImageView) {
view.setOnClickListener {
if (omokGame.isRunning()) gameRunning(index, view)
if (!omokGame.isRunning()) gameFinished()
}
}

private fun gameRunning(index: Int, view: ImageView) {
val stone = omokGame.getStone(Converter.indexToPosition(index))
val isSuccess = omokGame.placeTo(stone)
if (isSuccess) {
printStone(view, stone)
omokDbManager.updateOmokDatabase(stone)
omokGame.checkFinished()
}
showTurn(omokGame.currentColor)
}

private fun gameFinished() {
omokGame.getWinnerColor()?.let { printWinner(it) }
reset(boardCoordinateViews, omokGame)
showTurn(omokGame.currentColor)

Choose a reason for hiding this comment

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

지금 로직에서는 다음 바둑돌을 내려놔야만 게임이 종료된 것을 아는 것일까요?
게임 진행과 종료까지는 하나의 흐름으로 봐야 하지 않을까요?

돌을 클릭한다 -> 돌을 내려둔다 -> 게임이 종료되었는지 확인한다 -> 종료한다

Copy link
Author

Choose a reason for hiding this comment

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

리뷰어님의 의도를 정확히 알 수 있을까요?? 게임 진행과 종료까지는 하나의 흐름으로 봐야한다는게 이해가 잘 되지 않습니다. ClickBoard 함수 안에서 하나의 흐름을 실행시키고 있다고 볼 수 있지 않을까요??

Copy link

@laco-dev laco-dev Mar 27, 2023

Choose a reason for hiding this comment

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

보드를 클릭했을때 다음 클릭을 하지 않더라도 자동으로 게임이 종료되는 것을 생각했습니다.

돌을 내려둔 뒤, checkFinished()는 호출하지만 이어서 게임이 실제로 종료되는 것으로 전환 하는 것은 "다음 클릭" 이벤트가 발생한 시점으로 보여서요.

이 내용은 컨트롤러의 구현 방법에 따라 다를 수 있겠네요.
하나의 UI 이벤트가 결과적으로 상태를 변경하는 흐름이 되는 것을 기대하고는 했습니다.
아래 내용은 참고만 해주세요~

fun onClickBoard(...) [
   val placeResult = omokGame.placeTo(...)
   when (placeResult) {
      "게임 끝" -> printEnd()
      "게임 중" -> printRunning()
      "게임 ..." -> ...
   }
}

app/src/main/java/woowacourse/omok/OmokDbManager.kt Outdated Show resolved Hide resolved
Copy link

@laco-dev laco-dev left a comment

Choose a reason for hiding this comment

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

피드백 확인했습니다.

안드로이드 환경에서 구현하더라도 지금까지 해온 미션을 잘 살릴 수 있도록 잘 활용 해보시면 좋겠습니다.
오목 미션은 종료하겠습니다.

그동안 고생 많으셨습니다. 🎉

@laco-dev laco-dev merged commit b368905 into woowacourse:chws0508 Mar 27, 2023
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.

3 participants