-
Notifications
You must be signed in to change notification settings - Fork 143
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 2] 햄찌, 가마 #230
base: ic_11_hamzzi
Are you sure you want to change the base?
Conversation
…-3 의 경우에만 가능하도록 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 햄찌, 가마!
1주 동안 프로젝트 하시느라 고생 많으셨습니다!
제 실력이 부족하여 더 좋은 리뷰를 남겨드리지 못한게 너무 아쉽네요🥹
다음 프로젝트부터는 서포터즈와 현업 리뷰어분들의 리뷰를 받으실텐데, 다들 정말 좋은 리뷰를 남겨주시니 많이 배워가시면 좋을 것 같아요!
제가 현재 남긴 코멘트들은 수정해달라는 것이 아닌 단순히 제 생각을 말씀드리는 거라 필요하다고 생각하시는 부분들만 챙겨가시면 좋을 것 같아요🙂
조언을 얻고 싶은 점
저희 코드에서 runGameFirstStep() 함수에서 사용자 입력을 받는 부분을 함수 분리를 하고 싶었는데 중간에 while문을 제어하는 loopFlag가 존재하여 함수 분리를 하지 못했습니다. 이 부분에 대해서 코드적으로 조언을 얻고 싶습니다!
inout에 대해서 공부해보시면 도움이 되지 않을까 싶어요.
변수 및 함수 네이밍이 적절하게 이루어졌는지 알고 싶습니다.
네이밍은 저 또한 너무 어렵기에 조언 드리기가 어렵네요🥲
축약어는 최대한 지양하고, 외부 사람도 잘 이해할 수 있도록 Swift API Guidelines를 기반으로 팀끼리 잘 조율하면 된다고 생각해요.
컴퓨터의 패를 알 수 없어 이를 출력시키는 print문을 임의로 추가했습니다. 게임 진행에 그렇게 영향을 미치지 않는다고 판단했습니다. 이렇게 실행에 영향을 미치지 않지만 상황을 부차적으로 설명할 수 있는 출력문을 추가적으로 작성해도 좋을까요?
앞으로 다른 리뷰어 분들과 더 많은 프로젝트를 진행하실텐데, 기능을 추가하는 건 프로젝트 요구사항을 모두 준수한 후에 리뷰어와 상의하여 고려해보면 좋을 것 같아요🙂
저는 괜찮다고 생각합니다!
리뷰어님이라면 최초 실행 상태로 복귀 시키기 위해 재귀 혹은 반복문 둘 중에서 무엇을 사용하실지 궁금합니다!
재귀함수는 조건문보다 비교적 코드가 간결해질 수 있다는 장점이 있을 것 같은데, 스택오버플로우나 메모리 문제 등으로 완벽하게 제어할 수 없다면 최대한 지양하는게 좋다고 생각해요.
저는 완벽하게 제어할 자신이 없어서 반복문으로 구현하는 방식을 고려해보지 않을까 싶네요🥹
enum GameState { | ||
case rockScissorPaper | ||
case mukChiba | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 타입을 활용하여 어떤 게임을 실행할지 제어하는게 아닌, 단순히 명시적으로 보여주기 위함이라고 한다면 게임을 실행하는 함수명을 runRockScissorsPaper
, runMukChiBa
등으로 바꿔주는 것만으로도 충분할 것 같다고 생각이 드네요.
} | ||
|
||
var gameState: GameState = .rockScissorPaper | ||
var whoseTurn: GamePlayer = .computer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turn의 초기값이 computer인게 어색하다고 볼 수도 있을 것 같아요.
해당 변수를 옵셔널로 두거나 GamePlayer에 누구의 턴도 아닌 case를 추가한다면 좋을 것 같아요🙂
} | ||
} | ||
|
||
func printResult(gameResult: GameResult) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argument label을 추가해준다면 해당 함수를 사용하는 곳에서 가독성이 더 좋아질 수도 있을 것 같아요🙂
func printResult(of gameResult: GameResult) { }
printResult(of: gameResult)
enum GamePlayer: String { | ||
case player = "사용자" | ||
case computer = "컴퓨터" | ||
} | ||
|
||
enum GameResult { | ||
case draw | ||
case win | ||
case lose | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum을 활용하신 부분 너무 좋아요👍
print("\(whoseTurn.rawValue)의 턴입니다.") | ||
case .lose: | ||
whoseTurn = .computer | ||
print("\(whoseTurn.rawValue)의 턴입니다.") | ||
default: | ||
print("\(whoseTurn.rawValue)의 승리!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum의 rawValue를 활용하신 부분 너무 좋아요👍
추가로 CustomStringConvertible에 대해서도 알아보시면 좋을 것 같아요🙂
꼭 사용해야 한다는 것은 아닙니다!
func judgeRockScissorPaper(playerChoice: String, computerChoice: String) -> GameResult { | ||
switch (playerChoice, computerChoice) { | ||
case (_, _) where playerChoice == computerChoice: | ||
return .draw | ||
case ("1", "3"), ("2", "1"), ("3", "2") : | ||
return .win | ||
default: | ||
return .lose | ||
} | ||
} | ||
|
||
func judgeMukChiBa(playerChoice: String, computerChoice: String) -> GameResult { | ||
switch (playerChoice, computerChoice) { | ||
case (_, _) where playerChoice == computerChoice: | ||
return .draw | ||
case ("1", "3"), ("2", "1"), ("3", "2") : | ||
return .lose | ||
default: | ||
return .win | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
숫자만으로는 어떤 것을 기준으로 승패가 나뉘는지 명확하게 이해하기가 어려울 수도 있어서 choice또한 enum으로 정의해준다면 더 쉽게 이해할 수 있을꺼 같아요🙂
} | ||
} | ||
|
||
runGameFirstStep() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금은 함수만으로 기능을 구현했지만 나중에 시간이 되신다면 struct/class나 protocol을 활용하여 가위바위보와 묵찌빠를 두 가지 타입으로 나누어 구현해보시면 좋을 것 같아요🙂
func makeComputerChoice() -> String { | ||
let computerChoice = String(Int.random(in: 1...3)) | ||
return String(computerChoice) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 굳이? 라는 생각이 드는 부분이긴 한데
swift api guidelines에 따르면 factory method의 시작은 make로 시작해달라는 내용이 있어서 그 이외에 함수에는 최대한 make단어로 시작하지 않는 것으로 알고 있어요.
안녕하세요! 3조 햄찌, 가마입니다! 잘 부탁드립니다~ @carti1108
고민한 점
GameResult
enum을 추가했습니다.GameState
enum을 추가했습니다.GamePlayer
enum을 추가하고, 누구 턴인지 print문으로 출력하기 위함으로 한글로 된 문자열 rawValue를 사용하게 되었습니다.loopFlag
를 제어하는 것loopFlag
의 값을 변경하지 않고, continue를 통해 while문 초기로 돌아가게 설정했습니다. 게임 종료(0)일 때에는 return으로 while문을 끝내도록, 승패가 갈라진 경우에만loopFlag
의 값을 false로 변경하여 while문에서 탈출하도록 하였습니다.runGameFirstStep()
함수와 묵찌빠를 실행하는runGameSecondStep()
함수 각각에서 흐름 별로 각 기능을 분담하는 함수들을 호출케 하였습니다.isValidInput()
함수를 호출시켜 사용자가 입력한 값이 0, 1, 2, 3 중 하나인지 판별하게 했습니다.makeComputerChoice()
함수를 통해 랜덤으로 컴퓨터 패를 생성케 하였고, 해당 승부에 대한 결과를judgeRockScissorPaper()
함수를 호출하여gameResult
변수에 결과값을 받게 했습니다.gameResult
변수가 비긴 경우, 승패가 갈린 경우를 if문으로 나누고, 승패가 갈렸을 때 다음 묵찌빠로 넘어가도록runGameSecondStep()
함수를 호출했습니다.조언을 얻고 싶은 점
runGameFirstStep()
함수에서 사용자 입력을 받는 부분을 함수 분리를 하고 싶었는데 중간에 while문을 제어하는 loopFlag가 존재하여 함수 분리를 하지 못했습니다. 이 부분에 대해서 코드적으로 조언을 얻고 싶습니다!