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

가위바위보 게임 [STEP 1] honghoker #6

Open
wants to merge 2 commits into
base: rft_2_eunpyo
Choose a base branch
from

Conversation

honghoker
Copy link

@ryan-son
안녕하세요! 벌써 3주차라니 믿기지 않네요.
2주차를 계속 잡고있다간 끝이 없을거 같아 3주차 먼저 진행해서 PR 요청 드립니다 😅
새벽에 올리는거라 무음으로 해두셨길 바라면서.. 조심스레 올려봅니다.

미션 요구사항

TDD의 RED-GREEN-REFACTOR 세 단계를 지켜가며 기능을 구현해봅니다
UI와의 연동 없이 비지니스 로직만 구현합니다

양쪽이 낸 패의 승패 판결을 위한 기능을 TDD로 구현합니다
해당 타입, 메서드를 구현해가며 지속적으로 리팩터링 합니다
삼세판을 이기면 승리하는 기능을 TDD로 구현합니다
삼세판이 끝나고 승패가 갈리면 초기화 하는 기능을 TDD로 구현합니다
성능에 유리한 코드로 작성하도록 노력합니다
기획의 변경에 대해서 최대한 열린 코드로 작성해봅니다

진행하며 느낀 점

이번에 TDD를 처음 접하는데, RED-GREEN-REFACTOR 세 단계를 지키면서 하려니까 생각보다 너무 불편했습니다.
자꾸 평소 버릇처럼 여기저기 튀어나가더라구요. 잠뿐만이 아니라 다른 부분에서 저와의 싸움을 한거같아 재밌었습니다.

고민했던 점

이번에는 폴더링에 신경을 전혀 안쓰고 테스트에만 집중해서 작성해보았습니다. (너무 신경안쓴거 같기도해서 죄송해요 🥲)
기획의 변경에 대해서 최대한 열린 코드로 작성하란게 보여서 유저뿐만이 아니라 컴퓨터도 함께 점수 카운팅을 해주었습니다.

조언 부탁드려요!

객체미용체조나 SOLID 를 생각해서 최대한 분리해보려고 했는데, 아쉬운 점이 몇가지 있는거 같아요.

승리와 패배 판별

Scoreable은 말 그대로 점수 카운팅만 해주는 역할이므로, 승패를 판단하려면 게임을 하는 플레이어와 관련되어 있는 Gamerable에서 해야한다고 생각해서 현재는 Gamerable protocol의 extension에서 computed 프로퍼티인 isWinner와 isLoser를 통해 승리와 패배를 판별하고 있는데요.
그닥 마음에 안들어서 한참 고민해보았는데 더 좋은 방법이 잘 떠오르지 않습니다. 조금 더 아름다운 방법이 있을지 조언 부탁드립니다!

대결 후 점수 카운팅 방법

제일 마음에 안드는 부분입니다!

RPSGame 내부에서 battle 메서드를 통해 결과에 따라 점수를 카운팅해주고 있습니다.
카운팅해주는 값이 직접 지정되어 있기 때문에 별도의 메서드로 분리를 하는게 맞다고 생각하는데, Scoreable 프로토콜에서 정의한 점수 카운팅과 관련된 프로퍼티들이 get set이 둘다 가능하므로 프로토콜 기본 구현체를 통해 별도의 메서드로 분리를 한다고 해도 직접 값을 수정할 수 있기 때문에 의미가 사라지는거 같습니다.
이 부분도 다른 방향이 있을 지 조언 부탁드립니다!

Copy link

@ryan-son ryan-son left a comment

Choose a reason for hiding this comment

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

honghoker, 마지막 프로젝트까지 오셨네요. 기획 변경에 대응한다는 요구사항은 쉽지 않으셨을텐데 고생 많으셨습니다.

마찬가지로 본문에서는 질문만 다룰게요. 세부 내용은 코드에서 확인하시면 되겠습니다.

조언 부탁드려요!

객체미용체조나 SOLID 를 생각해서 최대한 분리해보려고 했는데, 아쉬운 점이 몇가지 있는거 같아요.

승리와 패배 판별

Scoreable은 말 그대로 점수 카운팅만 해주는 역할이므로, 승패를 판단하려면 게임을 하는 플레이어와 관련되어 있는 Gamerable에서 해야한다고 생각해서 현재는 Gamerable protocol의 extension에서 computed 프로퍼티인 isWinner와 isLoser를 통해 승리와 패배를 판별하고 있는데요.
그닥 마음에 안들어서 한참 고민해보았는데 더 좋은 방법이 잘 떠오르지 않습니다. 조금 더 아름다운 방법이 있을지 조언 부탁드립니다!

개별 코멘트에도 남겨두었지만, 점수 관리 책임을 가지는 타입을 두고 각자의 점수 상태를 관리하되 단판 가위바위보 결과를 전달받아 각자의 점수를 업데이트하고 요청을 받으면 승자를 반환하는 기능을 둘 수 있을 것 같습니다. 이 타입이 최종 승자를 가리는 로직을 캡슐화해서 전략 패턴으로 구성한 후, 이니셜라이저 또는 메서드를 통해 승부 전략을 전달받으면 해당 전략에 따라 승자를 다른 방식으로 가려낼 수도 있겠네요.

대결 후 점수 카운팅 방법

제일 마음에 안드는 부분입니다!
RPSGame 내부에서 battle 메서드를 통해 결과에 따라 점수를 카운팅해주고 있습니다.
카운팅해주는 값이 직접 지정되어 있기 때문에 별도의 메서드로 분리를 하는게 맞다고 생각하는데, Scoreable 프로토콜에서 정의한 점수 카운팅과 관련된 프로퍼티들이 get set이 둘다 가능하므로 프로토콜 기본 구현체를 통해 별도의 메서드로 분리를 한다고 해도 직접 값을 수정할 수 있기 때문에 의미가 사라지는거 같습니다.
이 부분도 다른 방향이 있을 지 조언 부탁드립니다!

이것도 점수 관리 책임을 가지는 타입에게 책임을 위임해보면 괜찮은 해법이 나올 것 같네요.

계속해서 화이팅입니다~! 궁금하신 점이 있으시면 언제든지 말씀해주세요!

Comment on lines +10 to +12
protocol HandStrategy {
func nextHand() -> Hand
}

Choose a reason for hiding this comment

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

가위바위보 결정 전략이니 HandDecisionStrategy라는 이름이 더 잘 어울릴 수 있겠네요.

Comment on lines +10 to +28
protocol Scoreable {
var wins: Int { get set }
var draws: Int { get set }
var losses: Int { get set }

mutating func reset()
}

struct Score: Scoreable {
var wins: Int = 0
var draws: Int = 0
var losses: Int = 0

mutating func reset() {
wins = 0
draws = 0
losses = 0
}
}

Choose a reason for hiding this comment

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

Scoreable 추상화로 인해 얻을 수 있는 이점이 당장 보이지는 않네요. 리셋시킬 때 특수한 로직이 포함된다는 요구사항이 추가된다면 포함해볼 수 있을 것 같습니다. 저는 개인적으로 프로토콜을 통해 프로퍼티를 요구할 때 외부에서 프로퍼티를 조작할 수 있는 권한을 주지 않기 위해 set은 되도록 선언하지 않습니다. 프로토콜에서 해당 프로퍼티가 꼭 외부에 드러나야할지를 고민해보는 것도 좋을 것 같습니다.

Comment on lines +30 to +56
protocol Gamerable: AnyObject {
var handStrategy: HandStrategy { get }
var currentHand: Hand? { get set }
var score: Scoreable { get set }

func reset()
func next()
}

extension Gamerable {
var isWinner: Bool {
score.wins >= 2
}

var isLoser: Bool {
score.losses >= 2
}

func next() {
currentHand = handStrategy.nextHand()
}

func reset() {
score.reset()
currentHand = nil
}
}

Choose a reason for hiding this comment

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

  1. 프로토콜의 이름으로 ~할 수 있다는 의미의 -able, -ible을 일반적으로 사용하지만, 명사 자체가 자격의 의미가 있는 경우 명사를 사용하는 것도 생각해봄직 합니다. 예를 들어, Collection, Sequence와 같은 프로토콜들이 있겠네요. 프로토콜을 명사로 짓는 경우 -Impl라는 표현도 빈번하게 사용합니다. 정 생각이 나지 않는다면 -Protocol이라는 이름을 사용해볼 수도 있습니다.
  2. Gamerable에서 꼭 필요하다고 생각되는 인터페이스는 resetnext 메서드라고 생각합니다. Gamerable을 준수하는 타입이 꼭 외부에 handStrategy를 노출해야할 필요는 없고, next 메서드가 Hand 인스턴스를 반환한다면 currentHand 프로퍼티를 유지하지 않아도 좋을 것 같습니다. scorereset은 함께 다녀야할 요소인데, 저는 Gamerable에서 관리하기보다 게임의 스코어를 관리하는 타입을 두고 각 플레이어의 점수를 관리하고, 승부를 가리는 로직을 갖게해서 이곳에 프로토콜 기본 구현으로 제공된 사항을 모두 위임할 수 있을 것 같습니다. honghoker의 생각은 어떠신가요?

Comment on lines +58 to +78
final class User: Gamerable {
var handStrategy: HandStrategy
var currentHand: Hand? = nil
var score: Scoreable

init(handStrategy: HandStrategy, score: Scoreable = Score()) {
self.handStrategy = handStrategy
self.score = score
}
}

final class Computer: Gamerable {
var handStrategy: HandStrategy
var currentHand: Hand? = nil
var score: Scoreable

init(handStrategy: HandStrategy, score: Scoreable = Score()) {
self.handStrategy = handStrategy
self.score = score
}
}

Choose a reason for hiding this comment

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

UserComputer는 게임을 플레이하는 동일한 수준의 인스턴스를 만들면 되니 꼭 타입이 분리될 필요는 없을 것 같습니다. Player와 같은 구조체로 선언해보는 것도 좋겠네요. class로 선언하신 특별한 이유가 있으셨나요?

Comment on lines +80 to +94
protocol RPSGameable {
var user: Gamerable { get }
var computer: Gamerable { get }

func battle(userHand: Hand, computerHand: Hand)
func playBestOfThree()
func reset()
}

extension RPSGameable {
func reset() {
user.reset()
computer.reset()
}
}

Choose a reason for hiding this comment

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

사용자와 컴퓨터가 Hand를 결정해 단판 승부를 낼 수 있고, 삼세판을 통해 최종 승자를 가릴 수 있다는 정도로 추상화가 되었으면 꼭 가위바위보여야할 필요는 없을 것 같습니다. HandGame과 같은 추상적인 이름을 사용해볼 수도 있겠네요.

Comment on lines +96 to +117
final class RPSGame: RPSGameable {
let user: Gamerable
let computer: Gamerable

init(user: Gamerable, computer: Gamerable) {
self.user = user
self.computer = computer
}

func battle(userHand: Hand, computerHand: Hand) {
switch (userHand, computerHand) {
case let (u, c) where u == c:
user.score.draws += 1
computer.score.draws += 1
case (.paper, .rock), (.rock, .scissor), (.scissor, .paper):
user.score.wins += 1
computer.score.losses += 1
default:
user.score.losses += 1
computer.score.wins += 1
}
}
Copy link

@ryan-son ryan-son Mar 19, 2024

Choose a reason for hiding this comment

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

  1. 이전 프로젝트에서 다뤄본 전략 패턴을 활용한다면 승부를 결정할 때 구현한 전략에 따라 다른 방식으로 낼 수 있을 것 같네요. 기획 변경에 대응할 수 있는 하나의 포인트일 것 같습니다.
  2. 이전에 말씀드린 내용처럼 점수를 관리하는 역할을 수행하는 타입이 있다면 해당 역할을 하는 인스턴스에 승부 결과를 알려주고 이를 통해 점수 반영, 그리고 최종 승자 결정과 같은 역할을 위임할 수 있을 것 같습니다.

Comment on lines +15 to +20
override func setUpWithError() throws {
sut = RPSGame(
user: User(handStrategy: RandomHandStrategy()),
computer: Computer(handStrategy: RandomHandStrategy())
)
}

Choose a reason for hiding this comment

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

  1. 사용자와 컴퓨터가 수를 결정하는 전략이 무작위이므로 테스트를 하기 어려울거에요. 이 경우에는 HandStrategy를 만족하는 Mock을 만들어 상황을 의도적으로 예측 가능하게 만들어 테스트를 진행할 수 있습니다. 테스트 케이스마다 각자가 내기를 원하는 수를 선택해주고 대결을 시킬 수 있습니다.
struct MockHandStrategy: HandStrategy {
    var next: Hand?

    func nextHand() -> Hand {
        return next ?? .rock
    }
}
  1. 공식문서에서는 setUpWithError, tearDownWithError의 super 동작에 대해 언급한 바는 없지만, 만약 정의된 부모의 동작이 있다면 수행해주기 위해 super 메서드를 호출하는 것도 좋아보입니다.

Comment on lines +26 to +38
func test_사용자가_가위를_내고_컴퓨터가_가위를_내면_비긴다() {
guard let sut else { return XCTAssertNotNil(sut) }

// given
let user: Hand = .scissor
let computer: Hand = .scissor

// when
sut.battle(userHand: user, computerHand: computer)

// then
XCTAssert(sut.user.score.draws == 1)
}

Choose a reason for hiding this comment

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

  1. 모든 수에 대해 테스트 케이스를 만들면 테스트 메서드 수도 많아져야 하겠네요. 사용자와 컴퓨터 각각 3가지 수를 낼 수 있으니 9가지 조합에 대해 예상된 승패 결과를 놓고 로직을 검증해보는 것도 좋은 방법일 것 같습니다.
  2. battle 메서드를 프로토콜의 요구사항으로 만들어 외부에 노출시키기는 했지만, 단판 승부를 시키는 인터페이스가 외부에서 사용되어야할 케이스가 있을까요? 단판 승부를 위한 것이라면 외부에서 사용자와 컴퓨터의 수를 전달할 수 있는 인터페이스를 제공하기보다 사용자와 컴퓨터의 수는 각자의 HandStrategy에 의해 결정되고, battle만 시킬 수 있는 방법을 제공해도 좋을 것 같아요.

Comment on lines +162 to +163

func test_사용자가_삼세판을_졌을때_패배한다() {

Choose a reason for hiding this comment

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

비긴 후에 게임이 계속 진행되는지, 유효하지 않은 수(nil)를 냈을 때 의도한 에러가 발생하는지와 같은 테스트도 추가해볼 수 있을 것 같습니다.

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.

None yet

2 participants