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] 웅, 메네 #160

Merged
merged 15 commits into from
Aug 24, 2022

Conversation

JaeKimdev
Copy link

묵찌빠게임 [STEP 1] 웅, 메네

@derrickkim0109

안녕하세요 데릭! STEP 1 완료하고 PR 드립니다.

처음 제가 folk 해온 프로젝트에 Collaborator 설정을 해주지 않고 커밋을 했더니 웅이 pull했을 때 main 브랜치만 보여지는 현상이 있었고, git branch -a 명령으로 숨겨진 브랜치를 확인하여 이동하고 push했을 때 억세스 할 수 없다는 에러메세지가 보여졌습니다. 그래서 처음에 제가 2번의 commit을 했는데 나머지 부분은 번갈아가며 커밋하였습니다. 다음부터는콜라보레이터 등록 잘 하겠습니다. 😭

Enum을 사용해서 가위, 바위, 보 Hand 타입을 설정하고 CaseIterable 프로토콜을 이용하여 랜덤 컴퓨터 핸드 값을 생성하도록 구현해보았습니다.

STEP 2에서 묵찌빠 게임을 하면서 유저 인풋을 한번 더 받아야 하는 것으로 알고 있어서, 유저인풋을 받아 Int로 반환하는 함수를 분리하고 싶었으나, inputNumber에서 0, 1, 2, 3이 떨어지고 그 값을 받아 처리할때 계속해서 0을 어떻게 처리해야 할 지, 고민하였습니다.

사용자 핸드를 반환하는 makeUserHand 함수에서도 0,1,2,3이 들어오기 때문에 1,2,3에 각각의 결과를 switch를 사용하여 반환하고 싶었지만 Hand 타입을 반환하는 문제로 0을 처리할 수 없어서 if로 변경해서 사용을 하였고 함수 바깥쪽에서 0번 처리를 먼저 해주고 inputNumber == 3은 else 문을 사용하여 구현하도록 수정하였습니다.

컴퓨터 핸드와 유저 핸드를 비교하는 checkResult 함수에서도 숫자로 비교하는 것보다 Hand.scissorHand.rock을 비교하는 방식이 좀 더 직관적일 수 있을 것 같아 이렇게 구현하여 보았습니다.

고민했던 점

  • 유저입력 값에 대한 오류처리를 어떻게 해줘야 할 지에 대해서 고민했습니다.
    (do-catch와 guard 문을 고민한 결과, 결국 간단하게 할 수 있는 guard문을 채택하여 작성하였습니다.)

조언을 얻고 싶은 부분

  • func makeUserHand 의 반환값을 -> Hand로 지정 해줬는데 0을 입력했을 때

    저희는 switch를 먼저 시도했다가 default를 해결하지 못하고 0을 먼저 제거한 값을 if 로 받아서 구현하였는데 저희가 사용한 방법보다 좋은 방법이 있을까요?

  • func gameStart 의 출력문 & guard let 문장을 분리해주려고 시도하였으나, return 값을 요구하여 분리해주지 못하였습니다.
    (혹시 분리 할 수 있는 해결 방법이 있을까요?)

    gameStart 에서 유저입력 부분을 함수로 분리하여 추후 재사용하려고 생각했는데, 0 처리가 더 복잡해질 수 있을 것 같아 유지하는 것으로 결정했습니다.

Copy link

@derrickkim0109 derrickkim0109 left a comment

Choose a reason for hiding this comment

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

@JaeKimdev @iOS-Woong
안녕하세요! 여러분
이번 1주동안 묵찌빠 게임 프로젝트를 리뷰하게되어 영광입니다🙌
리뷰를 드리는것은 정답지가 아닌 질문들이라고 생각해주시면 좋겠습니다!
만약 리뷰 과정에서 여러분들의 의견이 더 적절할것 같으면 언제든지 코멘트로 말씀해주시고 밀고 나아가시면 됩니다! 😀

따로 말씀 드리자면 재귀함수에 대해서도 찾아보셨으면 좋겠습니다.!

그럼 리뷰를 드려보겠습니다!

칭찬 드리고 싶은 부분

  • 커밋을 단위별로 잘 나눠 페어로 작업해주신점 💯
  • 들여쓰기 2번을 초과하지 않고 작업해주신점 💯
  • enum 타입 사용하여 작업해주신점 💯

궁금하신 부분에 대한 저의 의견

고민했던 점

유저입력 값에 대한 오류처리를 어떻게 해줘야 할 지에 대해서 고민했습니다.
(do-catch와 guard 문을 고민한 결과, 결국 간단하게 할 수 있는 guard문을 채택하여 작성하였습니다.)

  • 오류 처리에 대한 부분은 do-catch와 guard 문 중에서 고민하셨군요.!ㅎㅎ
    하지만 do-catch가 사용될 때와 guard 문이 사용될 때는 엄연히 구분되는 부분이 있습니다.
    do-catch와 guard 문은 어떨 때 사용 하는 것일까요?

저의 생각에 대해서 조금 말씀 드리자면 do-catch문은 현재 상황에서는 사용될 이유가 없습니다. (유저 입력 값에 대한 처리용으로 말씀드리는 것입니다.)

조언을 얻고 싶은 부분

func makeUserHand 의 반환값을 -> Hand로 지정 해줬는데 0을 입력했을 때

저희는 switch를 먼저 시도했다가 default를 해결하지 못하고 0을 먼저 제거한 값을 if 로 받아서 구현하였는데 저희가 사용한 방법보다 좋은 방법이 있을까요?


func gameStart 의 출력문 & guard let 문장을 분리해주려고 시도하였으나, return 값을 요구하여 분리해주지 못하였습니다.
(혹시 분리 할 수 있는 해결 방법이 있을까요?)


gameStart 에서 유저입력 부분을 함수로 분리하여 추후 재사용하려고 생각했는데, 0 처리가 더 복잡해질 수 있을 것 같아 유지하는 것으로 결정했습니다.

  1. 해당 내용은 아래에 switch 문을 작성해 두었습니다. 커스텀 enum을 통해 rawValue로 값을 구분해 보세요.
  2. 두 번째 질문의 경우 조금 더 명확한 설명이 필요해 보입니다.
  3. 현재 프로젝트에서는 유저 입력 부분을 따로 재사용을 위해 뺄 필요는 없어 보입니다.(step01)


import Foundation

enum Hand: CaseIterable {

Choose a reason for hiding this comment

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

enum을 통해서 가위, 바위, 보를 구현해 주셨군요!! 너무 잘하셨습니다. 또한 이름을 Hand로 지으시다니.. 저는 그냥 가위바위보 이렇게 지었던 기억이 있는데.. 네이밍 너무 좋습니다.

해당 타입을 파일로 분리해주세요.

Copy link
Author

Choose a reason for hiding this comment

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

해당 타입 파일로 분리해보겠습니다.

Choose a reason for hiding this comment

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

감사합니다!
질문이 있는데 이곳에 답변주지않으셔도됩니다! (DM으로 드릴게요)
보통 클래스나 구조체, 열거형의 경우 - 관리차원에서 보통 소스파일을 새로 하나 만들어 그곳에서 관리를 하는게 보편적인가요?

Comment on lines 16 to 17
var gameStopCheck: Bool = true
let randomComputerHand: Hand = Hand.allCases.randomElement().unsafelyUnwrapped
Copy link

@derrickkim0109 derrickkim0109 Aug 23, 2022

Choose a reason for hiding this comment

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

위의 상수,변수는 전역으로 있을 필요가 없어 보입니다. gameStopCheck에 대한 부분은 아래 반복문에서 다시 말씀 드리겠습니다. 또한 네이밍이 조금 Bool 타입의 네이밍과는 맞지 않아 보입니다.
네이밍 강연 - 노수진 iOS 개발자
ps. 노수진씨는 iOS 개발자로써 굉장히 유명한 분이십니다.

Copy link
Author

Choose a reason for hiding this comment

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

네~ 말씀하신대로 전역으로 있을 필요가 없겠네요;
함수 내부로 이동하고 Bool 타입 네이밍 웅과 함께 좀 더 고민해보겠습니다.

Choose a reason for hiding this comment

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

https://mousy-astronaut-c9d.notion.site/Naver-54f7423f963b4b85bd71d41e2ee828c8
강의가 알차네요! 메서드 부분을 네이밍하는 부분에선 아직 이해가 가지않는 부분도 많이있지만,

  • 만약(이번엔 사용안하기로했지만), gameStopCheck 라는 Bool 타입의 변수명을 새로 짓는다면 isComplete (???) 같은 변수명이 좋겠네요.

var gameStopCheck: Bool = true
let randomComputerHand: Hand = Hand.allCases.randomElement().unsafelyUnwrapped

func makeUserHand(of inputNumber: Int) -> Hand {

Choose a reason for hiding this comment

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

해당 메서드를 숫자가 아닌 enum타입으로 매직넘버를 받아 올 수 있다면 어떨까요?
네이밍 또한 이해하기 쉽지가 않습니다. 유저 손의 넘버를 만드는 것이 아니라 결과 값(가위, 바위, 보)을 나타낼 수 있는 네이밍이면 좋을 것 같아요. 네이밍을 작성할 때 리턴되는 값이 무엇인지도 같이 생각해보면 좋을 것 같습니다.

Copy link
Author

@JaeKimdev JaeKimdev Aug 23, 2022

Choose a reason for hiding this comment

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

parameter와 argument lable 이름 웅과 좀 더 고민해보겠습니다.
말씀하신대로 inputNumber를 그대로 쓰지 않고 enum 타입으로 반환하거나 - 처리를 다른 쪽에서 해야겠네요,
현 구조를 유지하면서 makeUserHand(using: inputNumber)를 이용할 수 있을 것 같은데 웅과 고민해보겠습니다.

Copy link

@iOS-Woong iOS-Woong Aug 23, 2022

Choose a reason for hiding this comment

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

  1. 해당 메서드를 숫자가 아닌 enum 타입으로 값을 가져오는 방법 고민해보기
  2. 유저손(결과값:가위or바위or보) 로 더 쉽게 알아 볼 수 있는 네이밍으로 한번 고민해보겠습니다

let randomComputerHand: Hand = Hand.allCases.randomElement().unsafelyUnwrapped

func makeUserHand(of inputNumber: Int) -> Hand {
if inputNumber == 1 {

Choose a reason for hiding this comment

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

매직넘버는 inputNumber == 1 부분에서 숫자 1에 해당 되는 값을 타입으로 바꾸는 것을 말합니다. 또한 if 문 보다는 switch 문이 조금 더 깔끔할 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

저희가 고민했던 점이
switch 문을 사용 시 inputNumber에 0, 1, 2, 3 값이 들어 올 수 있어서
Hand 타입으로 반환할 때 default를 고민해야 �하는 문제였는데요

func makeUserHand(of inputNumber: Int) -> Hand {
    switch inputNumber {
    case 1:
        return Hand.scissor
    case 2:
        return Hand.rock
    case 3:
        return Hand.paper
    default:
        return Hand ? <- 요부분 처리를 어떻게 해야 할까요;
    }
}

다른 방법으로는 if 사용시와 같이 0을 제외하고 입력값을 처리해줄 수 있을 것 같습니다.

func makeUserHand(of inputNumber: Int) -> Hand {
    switch inputNumber {
    case 1:
        return Hand.scissor
    case 2:
        return Hand.rock
    default:
        return Hand.rock
    }
}

리뷰해주신 내용 적용하면서 구조를 조금 더 고민해보도록 할께요

Copy link

@derrickkim0109 derrickkim0109 Aug 23, 2022

Choose a reason for hiding this comment

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

func makeUserHand(of inputNumber: Int) -> Hand {
    guard let result = Hand(rawValue: inputNumber) else {
        return .none
    }
    
    return result
}

이렇게도 적용해 볼 수 있을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

알려주신 방법을 사용하려고 하면
Type 'Hand' has no member 'none' 라는 오류 메세지가 보여지고 사용할 수가 없습니다.
enum에 none 값이 없다는 이야기인 것 같은데 어떻게 하면 사용할 수 있을까요...ㅠㅠ 😭
enum에 none 값을 설정한다면 랜덤컴퓨터 핸드에 none이 포함되어 찍히는 문제가 있습니다. 진퇴양난이네요

Copy link
Author

Choose a reason for hiding this comment

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

일단 Hand 타입에 none 을 추가하고 컴퓨터 핸드와 사용자 핸드 값을 만들 때 1..3 범위만 사용하도록 처리하였습니다.

Choose a reason for hiding this comment

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

네 그렇게 사용하시는 거 맞습니다 ㅎㅎ

}
}

func checkResult(computerHand: Hand, userHand: Hand) {

Choose a reason for hiding this comment

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

해당 부분은 결과를 체크한다기 보단 두 값을 비교한다는 의미의 네이밍이 적절해 보입니다. 이기고 지고를 같이 프린트하기 보단 비교된 값을 통해 win / lose / draw를 표현 할 수 있는 enum타입을 만들어서 Return하면 좋아 보여요. Return 후 그 결과 값이 print 되면 흐름이 맞을 것 같습니다.

요약하자면 두가지 기능을 하는 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

컴퓨터 Hand와 사용자 Hand를 비교하는 compareHand 함수와 반환된 Result 값의 rawValue를 출력하는 displayResult 함수로 분리하였습니다.

print("비겼습니다!")
} else if computerHand == Hand.scissor && userHand == Hand.rock {
print("이겼습니다!")
gameStopCheck = false

Choose a reason for hiding this comment

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

해당 변수는 제거하셨으면 좋겠습니다.

}

func gameStart() {
while gameStopCheck {

Choose a reason for hiding this comment

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

true/false를 통해서 게임을 시작/종료 시키기 보단 정확하게 나올 수 있는 결과 값을 통해서 게임 시작유무를 결정해 주세요. 결과 값이란 위에서 설명 드린 enum을 통해서 표현하시면 되겠습니다.

Copy link
Author

Choose a reason for hiding this comment

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

처음에 시도했던 방법은 while 문 뒤에 조건을 Result.win || Result.lose를 사용해보았는데, Bool 타입으로 변환될 수 없는 값이라 사용할 수 없다는 에러를 확인하고는 재귀함수를 사용하여 반복할 수 있도록 수정해 보았습니다.

while gameStopCheck {
print(randomComputerHand)
print("가위(1), 바위(2), 보(3)! <종료 : 0> :", terminator: " ")
guard let inputNumberString = readLine(), let inputNumber = Int(inputNumberString),

Choose a reason for hiding this comment

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

,로 띄워진 부분은 개행 처리가 필요해 보입니다.

Choose a reason for hiding this comment

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

결과 값을 enum처리하면 따로 Int로 타입 변경을 하실 필요가 없습니다. enum에 대해서 조금 더 공부해 보세요!

print(randomComputerHand)
print("가위(1), 바위(2), 보(3)! <종료 : 0> :", terminator: " ")
guard let inputNumberString = readLine(), let inputNumber = Int(inputNumberString),
inputNumber >= 0, inputNumber < 4 else {

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.

switch문 사용하여 inputNumber 유효성 체크하도록 수정해보았습니다.

print("잘못된 입력입니다. 다시 시도해주세요.")
continue
}
if inputNumber == 0 {

Choose a reason for hiding this comment

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

결과 값을 switch 문으로 변경하여 enum화 하시면 조금 더 명확한 코드를 짜실 수 있습니다.
다른 사람이 내 코드를 봤을 때 의미전달이 정확히 될지를 생각해보시면 좋을 것 같습니다.

switch result {
case .rock, .paper, .sissor:
// 원하는 코드
case .none: 
// 게임 종료
}

result는 특정 enum 타입으로 가정합니다.

@JaeKimdev
Copy link
Author

코멘트 확인하고 수정해보았습니다.

변경사항으로는

  • 전역변수를 모두 제거하였습니다.
  • Hand 타입에 .none을 추가하여 makeUserHand함수에서 사용할 수 있도록 하였습니다.
  • Hand 타입에 none이 추가되어 랜덤컴퓨터 핸드 생성 시 Hand(rawValue: Int.random(in: 1...3))의 값만 사용하도록 수정하였습니다.
  • Result enum을 추가하여 win, lose, draw 결과 출력과 반복 로직에 사용할 수 있도록 정의하였습니다.
  • checkResult함수를 컴퓨터 핸드와 사용자 핸드를 비교하여 Result를 반환하는 compareHand 함수와 반환받은 Result enum의 rawValue를 출력하는 displayResult함수로 분리하였습니다.
  • startGame 함수에서 유저 입력을 받아서 switch문을 사용하여 유효성 검사하고, 재귀함수를 이용하여 반복되도록 수정하였습니다.

확인 부탁드립니다.
감사합니다. 👍

@JaeKimdev
Copy link
Author

데릭 아침에 알려주신 방법으로 조금 더 리팩토링 해 보았습니다.
compareHand 함수에서 튜플로 값을 받아서 승/패/비김 처리를 하도록 변경하였고 - none case 처리를 위해 draw 는 디폴트로 받았습니다.
makeUserHand함수는 삭제하고 startGame 함수에서 사용자 입력값을 바로 Hand 타입으로 받아서 분기 처리하도록 변경했습니다.

조금 더 생각해 봐야 할 점은
사용자 핸드와 컴퓨터 핸드를 비교하는 함수 이름을 변경했는데 , 호출시의 가독성을 위해서는
display(of: gameResult) 라고 호출하는 것이 읽기에 자연스러울 것 같은데, 함수 이름이 display인것이 괜찮은지 궁금합니다.
다른 방법을 생각해 보았으나, Result 타입을 뜯어고쳐야 할 것 같아 일단 좀 더 고민하면서 STEP 2 진행해보겠습니다.

확인하시고 merge 부탁드립니다.
꼼꼼하고 친절한 리뷰 갑사합니다. 데릭! ❤️ 👍

@derrickkim0109 derrickkim0109 merged commit f84b79f into yagom-academy:ic_7_jaekimdev Aug 24, 2022
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

3 participants