묵찌빠 게임 [STEP 1] Ari, 알라딘 #82

leeari95 · 2021-10-12T02:54:23Z

Soll !! 안녕하세요. 첫번째 스텝 PR 보내드립니다~ 🥰

고민했던 것

들여쓰기를 초과하지 않도록 많은 고민을 했습니다.
코드를 읽는 사람이 편할 수 있도록 리팩토링 하는 법에 대해서 고민했습니다.
Naming 파트 부분을 참고하여 많은 고민을 했습니다.
메소드가 기능 별로 한가지 일만 할 수 있도록 분리하는 것에 대해서 많은 고민을 하였습니다.
순서도를 세세하게 작성하여 코드의 흐름을 파악할 수 있도록 고민해보았습니다.

궁금했던 것 / 조언을 얻고 싶은 부분

순서도를 세부적으로 적기 위해서 분리해보았는데 좀 더 적절한 방법, 더 나은 방법이 있는지 조언을 얻고 싶습니다.
Naming을 제대로 하였는지, 적절한지 조언을 얻고 싶습니다.
메소드명을 지을 때 파라미터명과 연결해서 지어주는 것이 나은건지? 아니면 메소드명에 한꺼번에 지어주는 것이 알맞은지 헷갈립니다.
- ex) validate(userInput:)
  validateUserInput(_ input:)
- ex) getUserInput()
  get(userInput:)

🤔 getUserInput() 메소드 네이밍

private func getUserInput() -> String? {
        if let userInput = readLine(), validate(playerNumber: userInput) {
            return userInput
        }
        print("잘못된 입력입니다. 다시 시도해주세요.")
        return nil
    }

해당 메소드의 확장성을 고려해서, input이라는 파라미터를 추가하고, default 값으로 readLine() 을 설정하면 어떨까라는 생각을 했습니다.

private func getUserInput(input: String? = readLine()) -> String? {
        if let userInput = input, validate(playerNumber: userInput) {
            return userInput
        }
        print("잘못된 입력입니다. 다시 시도해주세요.")
        return nil
    }

이런식으로 바꿔보면 어떨까라는 생각을했는데, 여기서 또 메소드 네이밍에 대해 고민을 하기 시작했습니다.
- validate(playerNumber) 이라는 메소드 네이밍을 고민했을때랑 마찬가지로,
  - getUserInput(input: String?) -> get(userInput: String?) 이 되는게 아닐까라는 생각을 했습니다.
  - 그런데 해당 프로젝트에서 쓰일때는 파라미터의 default 값으로 설정해놓은 readLine() 이 사용되기 때문에, getUserInput() 또는 get() 으로만 사용해야됩니다. 이런 경우에는 네이밍을 어떻게 해야되는지 잘 떠오르지가 않습니다.

🤔 isPlayerWin() 메소드의 가독성

숫자들을 프로퍼티로 별도로 만들어주어서 타입 내부 가독성을 높혀보았는데, 열거형을 활용하면 더 가독성이 높아질까요? 저희가 생각해보았을 땐 열거형을 썼을 경우 이름이 프로퍼티로 만들어주는 것보다 길어져서 오히려 가독성이 떨어지지 않나..? 이런 생각이 들었어요.

private func isVictory(_ playerNumber: String?, _ opponentNumber: String) -> Bool {
    let winningCaseA = playerNumber == "1" && opponentNumber == "3"
    let winningCaseB = playerNumber == "2" && opponentNumber == "1"
    let winningCaseC = playerNumber == "3" && opponentNumber == "2"
    return winningCaseA || winningCaseB || winningCaseC
}

처음에는 위와 같이 player가 승리하는 경우의 수를 나눠서 승패 판정을 했는데, 숫자로 표현된 패를 property로 변경했습니다. 또한, 이미 숫자를 property로 변경해서 가독성이 생겼는데, 굳이 case 별로 나눌 필요가 있나해서

private func isPlayerWin(_ playerNumber: String?, _ opponentNumber: String) -> Bool {
        return (playerNumber == scissor && opponentNumber == paper)
            || (playerNumber == rock && opponentNumber == scissor)
            || (playerNumber == paper && opponentNumber == rock)
    }

위와 같은 형태로 작성을 했는데, 가독성 측면에서 봤을때 이게 맞는 코드인지에 대한 의문이 생겼습니다.

🤔 randomNumber 프로퍼티 추가

computerNumber는 호출할때마다 값이 바뀌게 프로퍼티로 지정해주면 좋을 것 같다는 생각이 들었습니다.
```
var randomNumber: String {
        get {
            return String(Int.random(in: 1...3))
        }
    }

// repeat-while 문에서의 적용
repeat {
    computerNumber = self.randomNumber
```
그래서 이렇게 하면 해당 프로퍼티를 호출할 때마다 값이 바뀌는데, 잘 사용한건지 확신이 안서네요. Soll의 생각은 어떤지 궁금합니다!

아직 많이 부족하지만 잘 부탁드리겠습니다. 😊 🙏🏻

- 종료(0), 가위(1), 바위(2), 보(3)를 프로퍼티로 명시해주었음 - validate 메소드 내부 배열에도 String 대신에 프로퍼티로 대체해주었음 - isVictory 메소드 내부에 String 대신에 프로퍼티로 대체해주었음

soll4u · 2021-10-12T17:17:49Z

RockPaperScissors/RockPaperScissors/main.swift

+ print(Message.gameEnd)
+ }
+
+ private func getUserInput(_ input: String? = readLine()) -> String? {


인자 기본값을을 사용해서 메서드를 만드셨다니 대단합니다! 좋은 아이디어에요 👍

메서드 이름에 대해서는 저번에 디코로 잠깐 말씀드렸는데요, 두 가지 정도 생각나는 것이 있네요 😃

get 같은 Swift에서 사용하고 있는 예약어는 메서드에 잘 사용하지 않습니다!
get을 대체할 단어는 어떤게 있을까요? 상의해보시고 알려주세요

Swift API Design Guidelines 의
Compensate for weak type information to clarify a parameter’s role.
해당 내용을 보시면 참고가 될 것 같습니다 🙂

같이 고민해서 getUserInput 을 recieveUserInput으로 변경해보았습니다! 😃

soll4u · 2021-10-12T17:34:09Z

RockPaperScissors/RockPaperScissors/main.swift

+ private func isDraw(_ computerNumber: String, _ playerNumber: String?) -> Bool {
+ if computerNumber == playerNumber {


사용자의 숫자와 컴퓨터의 숫자를 비교하는 메서드를 만들 때,
파라미터들(computerNumber와 playerNumber) 순서의 기준이 있나요? 😃

헉.. 피드백 받고보니 이제까지 기준이 없었던 것 같아요. isPlayerWin에 맞춰서 player가 앞에 올 수 있도록 기준을 정해주었습니다.

soll4u · 2021-10-12T17:34:57Z

RockPaperScissors/RockPaperScissors/main.swift

+ return playerNumber == nil
+ }
+
+ private func isPlayerWin(_ playerNumber: String?, _ opponentNumber: String) -> Bool {


computerNumber라고 하지않고 opponentNumber라고 하신 이유가 있나요? ㅎㅎ

컴퓨터 뿐만 아니라 다른 상대도 올 수 있기 때문에 좀 더 포괄적인 표현을 적용해보았습니다. 😁

soll4u

안녕하세요! 두 분 정말 고생많으셨습니다 :)
2주동안 잘 부탁드립니다 ㅎㅎ

칭찬할점

외부에서 사용하지 않는 프로퍼티와 메서드에 접근제어자 설정을 해주셨네요!! 아주 좋습니다 👍
네이밍 고민을 많이 하셨다고 들었는데요, 덕분에 코드를 읽는데 수월했습니다 👍

궁금한 점에 대한 답변

순서도를 세부적으로 적기 위해서 분리해보았는데 좀 더 적절한 방법, 더 나은 방법이 있는지 조언을 얻고 싶습니다.

⇒ 승패판정에 대한 순서도를 따로 만들어주셔서 보기 편했습니다! 좋습니다 👍
Naming을 제대로 하였는지, 적절한지 조언을 얻고 싶습니다.
메소드명을 지을 때 파라미터명과 연결해서 지어주는 것이 나은건지? 아니면 메소드명에 한꺼번에 지어주는 것이 알맞은지 헷갈립니다.

⇒ 정답은 없습니다! 다만 Naming 가이드라인이나, 예약어를 피해서 메서드 이름을 지어주면 좋겠죠?
근거가 있다면 규칙을 정하고 조끼리의 컨벤션을 맞추는게 중요하다고 생각해요 ㅎㅎ
그런점에서 getUserInput(_:) 메서드와 validate(playerNumber:) 메서드는 다른 컨벤션이 적용된 걸로 보입니다. (많이 고민하신게 눈에 보이네요 😄)
코멘트를 달아두었으니 참고 부탁드려요!
isPlayerWin() 메소드의 가독성

⇒ 제 생각엔 "1", "2" 이렇게 String 타입을 하드코딩한 것보다는 프로퍼티 이름을 써준 것이 좋아보입니다. 구현 방식에 대해서는 다양한 아이디어가 있을 텐데, 콕 집어서 이게 제일좋다고 말씀드리기는 어려울 것 같아요 😅

추가적으로, RockPaperScissorsGame이라는 타입의 역할이 많아보여요!
어떻게 하면 역할별로 세분화된 타입을 구현할 수 있을까요? 😄

달아드린 코멘트를 바탕으로 팀원끼리 상의해보시고
주중에 디코에서 만나서 얘기해보면 좋겠네요~
고생하셨습니다!

soll4u · 2021-10-12T18:14:38Z

RockPaperScissors/RockPaperScissors/main.swift

+ private var randomNumber: String {
+ get {
+ return String(Int.random(in: 1...3))


연산 프로퍼티에서 get만 있다면 생략가능한 걸로 알고 있는데요,
10~12 라인과 다르게 명시해주신 이유가 궁금합니다 👀

앗.. 생각해보니 get을 생략할 수 있는데... 깜빡했어요! 😅

soll4u · 2021-10-12T18:15:55Z

RockPaperScissors/RockPaperScissors/main.swift

+ var description: String {
+ return rawValue
+ }


description이 case보다 상단에 있는 이유가 궁금합니다 :)

이 부분도 다시 생각해보니 case보다 아래에 위치해있는 것이 올바르다고 판단되어서 수정하였습니다. 감사합니다! ☺️

soll4u · 2021-10-12T18:18:33Z

RockPaperScissors/RockPaperScissors/main.swift

+ if let userInput = input, validate(playerNumber: userInput) {
+ return userInput
+ }


42~45번째 줄에서는 guard 를 사용하셨는데요,
guard let과 if let을 사용하는 기준이 어떻게 되시는지 궁금합니다 😀

guard는 블럭을 탈출할 때 기준으로 활용하고, if는 바인딩만 할 경우에 사용하는 기준으로 활용하고 있습니다.
근데 피드백 반영하여 코드를 수정하다보니 해당 부분은 if 보다는 switch가 좋을 것 같다는 판단으로 수정되었습니다!

soll4u · 2021-10-12T18:21:12Z

RockPaperScissors/RockPaperScissors/main.swift

+ }
+
+ private func validate(playerNumber: String) -> Bool {
+ let validInputs = [quit, scissor, rock, paper]


만약 메뉴가 더 늘어나게 된다면 어떻게 될까요?
아니면 가위바위보 게임에서 확장이 되어 낼 수 있는 패가 많아지면 어떻게 될까요?
지금의 코드라면 그때마다 이 함수를 찾아와서 프로퍼티를 추가해줘야 할 것 같아요 ㅎㅎ

어떻게 케이스들을 관리하고 자동으로 validate할 수 있을지 고민해보시면 좋을 것 같아요

해당 메서드는 피드백을 반영해서 없어졌습니다.
Soll 덕분에 불필요한 메서드를 지울 수 있게 되었어요. 감사합니다! 🥰

soll4u · 2021-10-12T18:30:30Z

RockPaperScissors/RockPaperScissors/main.swift

+ }
+
+ private func isWrong(playerNumber: String?) -> Bool {
+ return playerNumber == nil


return 이 생략 가능해보입니다 😃

엇.. 이 부분도 놓쳤네요! 수정해보았습니다! 📝

soll4u · 2021-10-12T18:33:20Z

RockPaperScissors/RockPaperScissors/main.swift

+ return false
+ }
+
+ private func isWrong(playerNumber: String?) -> Bool {


number가 nil인지 체크하는 메서드네요! 좋습니다.
isWrong보다 더 메서드가 하는 일을 명확히 나타낼 수 있는 이름은 없을끼요? 🤔

좀 더 명확하게 isWrongInput 으로 변경해주었습니다!

soll4u · 2021-10-12T18:36:42Z

RockPaperScissors/RockPaperScissors/main.swift

+ private let quit = "0"
+ private let scissor = "1"
+ private let rock = "2"
+ private let paper = "3"


입력가능한 숫자들을 저장하는데 열거형을 사용하지 않고 프로퍼티를 사용하신 이유가 있나요? 😃

다시 생각해보니 열거형으로 만들어 주는 편이 훨씬 나을 것 같다는 내용으로 의견을 나누고 따로 열거형으로 만들어보았습니다. ☺️

- isPlayerWin 메소드 파라미터 순서 변경 - isWrong 메소드명 변경 후 return 생략

- Message 내부 desctiption 프로퍼티 아래로 순서 변경 - RockPaperScissor 이름을 PlayerOption으로 변경 - startGame 메서드 내부에 있던 if문을 printGameResult 메서드로 분리함

- RockPaperScissorsGame에 있는 property에 private 접근제어 추가 - randomHand를 PlayerOption 내부로 이동

soll4u

커밋 이름은 RockPaperScissor 열거형 추가 인데
실제 반영된 내용에 변수명 리네임이 포함되어 있어요.

커밋의 단위에 대해서 고민해보시면 좋겠습니다. :)

leeari95 and others added 14 commits October 11, 2021 17:02

docs: README.md 파일에 flow chart 추가

569e423

docs: README.md 파일 내부 순서도 수정

83b1fcb

docs: README.md 파일 내부 순서도 재수정

a54f553

feat: 사용자의 입력/검증 기능 구현

0d04430

feat: 승패판정 기능 추가

dfa067a

feat: 게임 종료 기능 추가

80e5b0e

refactor: isVictory를 isPlayerWin으로 변경 후, 내부 코드 리팩토링

f254858

refactor: gerUserInput 메소드 default parameter 추가

64fa0bd

refactor: 메소드 순서 변경, main 메소드명을 startGame으로 변경

10a6f96

refactor: randomNumber 프로퍼티 생성 후 startGame 내부 수정

7f0bfa9

refactor: Message type 생성 후 리팩토링

fa3e881

refactor: 프로퍼티 접근제어 부여

f8ad6c3

docs: README.md 파일 내부 순서도 수정

156eba1

leeari95 marked this pull request as draft October 12, 2021 05:04

leeari95 marked this pull request as ready for review October 12, 2021 05:05

soll4u reviewed Oct 12, 2021

View reviewed changes

junbangg and others added 4 commits October 13, 2021 11:03

refactor: RockPaperScissor 열거형 추가

2460cb1

refactor: isPlayerWin, isWrong 메서드 리팩토링

bfb306e

- isPlayerWin 메소드 파라미터 순서 변경 - isWrong 메소드명 변경 후 return 생략

refactor: 승패판정 기능을 GameJudgment 구조체 분리

49b7a54

leeari95 requested a review from soll4u October 13, 2021 03:03

leeari95 force-pushed the 4-leeari95 branch from cfc0167 to f00e34a Compare October 13, 2021 06:45

refactor: RockPaperScissorsGame 리팩토링

64dae9a

- RockPaperScissorsGame에 있는 property에 private 접근제어 추가 - randomHand를 PlayerOption 내부로 이동

soll4u reviewed Oct 13, 2021

View reviewed changes

leeari95 changed the title ~~[STEP 1] Ari, 알라딘~~ 묵찌빠 게임 [STEP1 1] Ari, 알라딘 Oct 13, 2021

leeari95 changed the title ~~묵찌빠 게임 [STEP1 1] Ari, 알라딘~~ 묵찌빠 게임 [STEP 1] Ari, 알라딘 Oct 13, 2021

soll4u merged commit b36b4ac into yagom-academy:4-leeari95 Oct 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

묵찌빠 게임 [STEP 1] Ari, 알라딘 #82

묵찌빠 게임 [STEP 1] Ari, 알라딘 #82

leeari95 commented Oct 12, 2021 •

edited

Loading

soll4u Oct 12, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021 •

edited

Loading

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u left a comment •

edited

Loading

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021 •

edited

Loading

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u Oct 12, 2021

leeari95 Oct 13, 2021

soll4u left a comment

		private func isDraw(_ computerNumber: String, _ playerNumber: String?) -> Bool {
		if computerNumber == playerNumber {

묵찌빠 게임 [STEP 1] Ari, 알라딘 #82

묵찌빠 게임 [STEP 1] Ari, 알라딘 #82

Conversation

leeari95 commented Oct 12, 2021 • edited Loading

고민했던 것

궁금했던 것 / 조언을 얻고 싶은 부분

🤔 getUserInput() 메소드 네이밍

🤔 isPlayerWin() 메소드의 가독성

🤔 randomNumber 프로퍼티 추가

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leeari95 Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soll4u left a comment • edited Loading

Choose a reason for hiding this comment

칭찬할점

궁금한 점에 대한 답변

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leeari95 Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soll4u left a comment

Choose a reason for hiding this comment

leeari95 commented Oct 12, 2021 •

edited

Loading

leeari95 Oct 13, 2021 •

edited

Loading

soll4u left a comment •

edited

Loading

leeari95 Oct 13, 2021 •

edited

Loading