-
Notifications
You must be signed in to change notification settings - Fork 6
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
[Lv_1 - Lv_6] 게임 시작 부분, 값 비교 등 구현 - 박채현 #21
base: Chae-Hyun
Are you sure you want to change the base?
[Lv_1 - Lv_6] 게임 시작 부분, 값 비교 등 구현 - 박채현 #21
Conversation
- BaseballGame 클래스 내 게임 시작 메소드 구현 - 답 배열의 count가 일정 개수가 될 때까지 랜덤숫자 추출 반복, 동일한 수가 없을 때만 append - 게임 시작 메시지 출력
입력값이 올바르지 않은 경우 Error로 정의 입력값을 guard문으로 나누어 Error 던져주는 메소드 정의 do문에서 해당 Error 받아 올바르게 작동되는 경우 Strike/Ball 개수 파악, catch문에서 Error에 따른 오류메시지 출력
outCount 변수를 만들고 입력값 체크하는 반복문에서 포함되지 않을 때마다 +1 outCount가 입력한 자리 수와 동일할 경우 (= 모든 자리가 일치하지 않을 경우) 아웃 메시지 출력
- while문의 조건을 게임 상태가 on일 경우, 반복해서 값을 받고 답에 대한 스트라이크와 볼 개수 출력 - 자리와 숫자가 모두 맞는 strike 변수의 값이 총 입력 값과 동일할 경우 게임 상태를 종료로 변경 -> while문 종료
스트라이크, 볼, 아웃 카운트 출력 주석 처리
- 중복 값이 아닐 경우 배열에 붙이고 첫 번째 인덱스의 값이 0일 경우 제거 구현 했었으나 연속적으로 계속 0이 나올 경우 수없이 반복 - 처음에는 0을 제외한 범위로 랜덤을 돌린 후 값이 붙은 후에는 0을 포함한 범위를 주는 방법으로 구현
- 게임 시작 혹은 기록보기, 게임 종료 선택 가능 - 요구된 숫자 이외 다른 값 입력시 재입력 요구
- 기록을 관리하는 다른 클래스를 만들어 게임이 계속 진행되는 동안 회차와 시도 횟수를 저장 - Model 분리
- 정답 개수, 범위 등의 enum 상수 값으로 분리 - 역할이 많은 메소드 -> 기능 별도 메소드로 분리 - 기록 매니저 튜플 값에 레이블 추가 - 출력 메소드 복잡, 각 enum에서 메시지 출력 메소드로 인해 기능 과다 -> 출력 매니저 구조체 선언
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.
와 구조가 엄청 깔끔하네요!! 흐름이 눈에 보이는 것 같아요!!
메소드의 역할 비중이 너무 커서 나누고 싶다고 하셨는데, 각 기능별로 메소드를 나누거나 해당 기능을 수행하는 클래스나 구조체를 생성해보면 어떨까요??
또, GameStatus, Guidance 같은 열거형들은 바깥이 아닌 연관된 클래스나 구조체 내에 구현해서 사용해도 좋을 것 같아요!!
typealias
에 대해 찾아보시고 활용할 방법이 있을지 고민해보셔도 좋을 것 같네요😈
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.
고생 많으셨습니다~~!
너무 맛있는 초콜릿 시켜주셔서
좀더 리뷰 열심히 했습니다~!
// 기록 매니저 | ||
private var recordManager: RecordManager | ||
private var printManager: PrintManager | ||
init(recordManager: RecordManager, printManager: PrintManager) { |
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.
class 안에서 recordManager = RecordManager()와 같은 방식으로 작성했었어도 되었을 것 같은데, 외부에서 주입받으시는 이유가 궁금합니다~
|
||
// 게임 상태 | ||
private var gameStatus: GameStatus = .on { | ||
willSet(newStatus) { |
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.
willset을 활용해 case마다 print가 실행되게 해 주셨군요!
제 개인적인 생각으로는 변수에 값을 할당했을 때 print가 수행되는 구조는 직관적이지 못할 수 있을 것 같아요.
만약 gameStatus 변수가 다른 파일로 분리되어 있고, 코드를 처음 보는 누군가가 startGame 메소드를 읽고 있다면, gameStatus = .on 코드에서 무엇가 출력되겠다는 예상을 하기 어려울 것 같습니다!
|
||
case "3": | ||
gameStatus = .off | ||
break commandLoop |
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.
switch case에서 반복문을 break하기 위한 문법 사용 좋습니다~
tryCount += 1 | ||
|
||
// 예외 경우 | ||
} catch InputError.inputCountError { |
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.
각각의 error를 catch해서 printManager에 전달하는 방법 보다 좋은 방법이 있을 수도 있을 것 같아요!
만약 에러가 늘어나거나 줄어드는 등 변경된다면 catch문도 계속해서 변경되어야 하는 문제가 예상됩니다..!
|
||
|
||
// 랜덤 값 추출 | ||
private func newGame() -> [Int] { |
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.
랜덤 값을 추출하는 기능만을 가진다면, 함수명에 해당 기능을 더 잘 나타내는 이름을 붙이면 좋을 것 같아요!
return inputAsIntArray.map({ $0! }) | ||
|
||
} | ||
|
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 SomeStruct {
func someFunction() {
}
<-- 요기 빈 줄이 있을 때도 있고 없을 때도 있는데요, 이러한 컨벤션도 일관적으로 지켜주시면 더욱 깔끔한 코드가 될 것 같아요!
}
**/ | ||
|
||
enum InputError: Error { | ||
case inputCountError, |
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.
case에 접근할 때 이미 InputError 라는 이름에서 Error임을 알 수 있어서, 각 case에 있는 Error는 제거해도 될 거 같아요..! 근데 이 부분은 개인적인 취향이라 채현님이 좋다고 생각하는 컨벤션을 따르면 될 것 같아요!
func printError(error: InputError) { | ||
|
||
let message = switch error { | ||
case InputError.inputCountError: |
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.
이 부분도 LocalizedError 프로토콜을 사용하면 훨씬 코드를 줄일 수 있을 것 같아요!
|
||
struct RecordManager { | ||
|
||
private var record: [(round: Int, tries: Int)] = [] |
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.
상태를 private으로 선언해 주시는 부분 너무 좋습니다~
|
||
private var record: [(round: Int, tries: Int)] = [] | ||
|
||
mutating func recordSet(tries: Int) { |
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.
상태를 변경하는 경우에도 mutating func을 사용해 구조체를 사용하는 것이 class보다 좋을까요? (저도 잘 모름...)
기존 풀리퀘스트를 머지하기 전 푸시하여 한 번에 머지되어 버렸습니다...༎ຶ‿༎ຶ
브랜치 삭제 후 통으로 올리는 점 양해 부탁드립니다..(--)(__)
1. 신규 기능
2. 변경된 점
case
가 아닌static let
으로 수정하였습니다.3. 고민했던 점