-
Notifications
You must be signed in to change notification settings - Fork 137
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] safari, malrnag #93
Conversation
veryfyNumber -> checkNumber 네이밍 변경 Set타입을 이용하여 검증기능 수정
inputPlayNumber()함수 from, to, count 파라미터 추가, 재귀함수를 이용하여 조건에 부합할 때까지 반복
오타가 발견되여 수정
순서도 점수 개산 단계 기호 수정
미처 남기지 못했던 궁금했던 점이 있습니다. |
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.
safari
, malrnag
안녕하세요~
두번째 스텝까지 하느라 고생하셨습니다! 저는 해내지 못한걸 여러분들은 해내셨네요... 🤭
처음 동료와 협업하면서 많은 어려움이 있으셨을탠데 끝까지 해내신거 너무너무 축하드립니다! 🎉
이번에도 같이 고민해봐요!!! 🙏🏻
칭찬 드리고 싶은 부분
- 커밋 단위가 저번 스텝보다 가독성이 좋았어요 💯
- 입력값의 유효성 검사를 심플하고 완벽하게 해내셨네요. 💯
- 고차함수를 적극 활용해보셨네요. 💯
- 옵셔널을 활용해서 예외상황을 잘 구현해주셨네요 💯
고민했던 부분에 대한 저의 의견
1. while문 조건문을 현재 true가 아닌 다른 방식도 있는지 조언을 듣고 싶습니다.
- while문 내부 상단에 특정 조건이 되면
break
를 하도록 하는 건 어떨까요?- 특정 조건은.. 플래그 역할을 하는
Bool
타입 변수가 좋을 것 같아요.
- 특정 조건은.. 플래그 역할을 하는
2. 입력 받은 숫자 배열을 조건에 맞게 유효성을 검증하는 과정에서 한번에 반복문 없이 해결하는 부분에 대해 고민하였습니다.
- 와.. 이 부분 대박이에요. 교집합을 통해서 완벽하게 유효성 검사를 하고 있네요. 한수 배워갑니다. 👏🏻👏🏻👏🏻👏🏻
3. 중복으로 쓰이는 값들을 전역변수로 만들어보면 어떨까요?
- 와우.. 깊은 고민을 해보셨네요!👍🏻 저도 동의합니다! 보통 저도 중복된 값들은 따로 변수로 만들어서 이곳 저곳에서 사용할 수 있도록 해주는 편입니다. 😁
- 시간이 남는다면
네임 스페이스
에 대해 알아보시는 것도 좋을 것 같아요.
- 시간이 남는다면
그 외 저의 의견
실행예시가 다른 부분을 발견했어요.🔎
- 이 부분은
사용자 승리!
가 밑에 위치해있어야 하는 것이 아닌가요? 게임을 열심히 해보았는데, 이 부분 예시랑 다른 것 같아서 말씀드려봅니당 🤩- 컴퓨터가 승리했을 경우에도 출력 순서가 사진과 동일합니다.
네이밍도 중요하지만 메소드 순서도 고민해볼까요?
- 코드를 작성하는데 있어서 이름짓기도 중요하지만, 함수 순서도 중요하다고 생각합니다.
- 보통은 코드를 읽기 편한 흐름으로 함수 순서의 기준을 정한다고 합니다. 두분도 이 부분에 대해서 다시 한번 고민해보셨으면 좋겠습니다! 😊
- 저는 보통 호출되는 순서로 함수를 나열합니다!
두분 너무 고생 많으셨어요...!
자세한 피드백은 코멘트로 달아드렸는데, 혹시 의견이 다른 부분이나 궁금한 점은 바로바로 말씀해주세요~ DM을 주셔도 좋습니다!
switch inputMenuNumber { | ||
case "1": startGame() | ||
case "2": return | ||
default: print("입력이 잘못되었습니다."); continue |
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.
여기 모두 내려쓰기를 하지않고 한줄로 맞춘 이유가 궁금합니다~ 😄
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 구문의 코드가 길지않아 내려쓰기를 하지않았었는데 내려쓰기를 하는것이
가독성에 더 좋았을까요~?
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 Design Guidelines를 다시 정독해봐도 좋을 것 같아요.
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내부 코드가 길지않아 한줄로 작성해왔습니다!
그렇게 작성하면 전체적인 코드의 길이를 줄일수있기때문이었습니다! 가독성은 크게 고려하지 않고 작성했던 부분인것 같습니다!
코드를 읽는 입장에서는 코드가 다같이 붙어있는것 보다 오히려 내려쓰기를하여 한줄한줄 눈에 잘 들어오도록 작성하는게
가독성측면에서 더 좋은것 같습니다!
다시한번 정독 해보도록 하겠습니다!
while true { | ||
print("1. 게임시작\n2. 게임종료") | ||
print("원하는 기능을 선택해 주세요 : ", terminator: "") | ||
guard let inputMenuNumber = readLine() else { return } |
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.
옵셔널 바인딩에 실패하게 된다면 사용자에게 별다른 안내 없이 프로그램이 바로 종료될 것 같네요?
옵셔널 바인딩을 guard 뿐만아니라 switch로도 할 수 있는 사실을 알고 계실까요?
이걸 잘 이용한다면... guard를 쓰지않고도 switch만을 이용해서 해결해볼 수 있을 것 같아요.
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로 한번 작성해 보겠습니다.
감사합니다.
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 의 조건이 .some 이라면 상수를 선언해 값을 할당해줄수도 있으며 할당하지않고
특정 코드를 실행하도록 할수있다는것을 알게 되었습니다!
그리고 금요일 학습활동을 통해 알게된
옵셔널값과 논옵셔널값을 비교 할수있다는게 기억이나 바인딩작업을 해주지 않아도 된다는것을 깨달았습니다,..!!
바인딩하지 않아도 입력값이 있다면 "1", "2" 인지 검사할수 있게되고 값이nil 이라면 default 를 실행하도록 변경했습니다!
print("중복 숫자는 허용하지 않습니다.") | ||
print("입력 : ", terminator: "") | ||
guard let inputPlayNumbers: [String] = readLine()?.components(separatedBy: " ") else { return nil } | ||
guard let playNumbers: [Int] = checkPlayNumber(from: from, to: to, count: count, playerNumbers: inputPlayNumbers) else { return inputPlayNumber(from: from, to: to, count: count) } |
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.
옵셔널을 잘 활용하고 계신 것 같아요. 최고에요! 😆🙌🏻
그치만 이 부분은 코드가 길어서 가독성이 떨어지는 것 같아요.
보통 저는 이럴 때 아래처럼 내려써주는데요. 가독성을 위해서 네이밍 뿐만 아니라 코딩 컨벤션도 매우매우 중요합니다!
두분이서 같이 이야기나눠보셨으면 좋겠네요.
개발자는 가로보다 세로로 보는 것을 더 좋아한다고 하네요 😁
guard let playNumbers: [Int] = checkPlayNumber(from: from, to: to, count: count, playerNumbers: inputPlayNumbers) else { return inputPlayNumber(from: from, to: to, count: count) } | |
guard let playNumbers: [Int] = checkPlayNumber( | |
from: from, | |
to: to, | |
count: count, | |
playerNumbers: inputPlayNumbers | |
) else { | |
return inputPlayNumber(from: from, to: to, count: count) | |
} |
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.
중요한 부분을 놓쳤네요...😓
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 playNumbers | ||
} | ||
|
||
func checkPlayNumber(from: Int, to: Int, count: Int, playerNumbers: [String]) -> [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.
이 함수가 from
to
라는 파라미터를 가져야만하는 이유가 궁금합니다. 🤔
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.
Set에 사용될 매개변수로 넣었는데 Range나 아님 Set자체 또는 배열만 가져갈 방법이 없는지 고민해보겠습니다.
func checkPlayNumber(from: Int, to: Int, count: Int, playerNumbers: [String]) -> [Int]? { | ||
let range: Set<Int> = Set<Int>(from...to) | ||
let veryfyNumbers: [Int] = playerNumbers.compactMap{ Int($0) } | ||
guard playerNumbers.count != count || range.intersection(veryfyNumbers).count != count else { return veryfyNumbers } |
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.
Set을 잘 활용해보셨네요. 교집합을 써서 사용자의 숫자가 제대로 된 숫자인지 정확히 검사를 하네요.. 멋집니다!👍🏻
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에서 사용한 Set이 생각이 나서 시도해 보았는데 운이 좋게 잘 맞았던 것같습니다.
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~9 이며 중복된 수는 없습니다. | ||
|
||
### Flow Chart | ||
<img src = "https://user-images.githubusercontent.com/91936941/153625713-897ce5db-4f89-43be-a6a8-9d88baaa180b.png" width="500px"> |
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.
와... 순서도 너무 깔끔하게 변신했는데요?
최고에요!!!! 👍🏻
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.
감사합니다. 순서도의 중요성을 아리의 프로젝트를 눈팅하며 많이 배웠습니다.
repeat { | ||
gameCount -= 1 | ||
guard let playerNumbers: [Int] = makeRandomNumber(to: 1, from: 9, count: 3) else { return } | ||
guard let playerNumbers = inputPlayNumber(from: 1, to: 9, count: 3) else { return } |
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.
타입 어노테이션이 제거된 이유가 궁금합니다!
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.
엇!! 코드를 계속 수정하는 상황에서 누락이 된것 같습니다. 함수의 리턴값을 받는 상수 또는 변수는 최대한 타입 어노테이션을 써주는 방향으로 했는데 말이죠😢
README.md
Outdated
|
||
### Step1 | ||
|
||
### 고민한점 |
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.
나머지 부분도 화이팅이에요!!! 💪🏻💪🏻
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.
리드미 화이팅..!! 아리 리드미보고 많이 배웠습니다!!
let veryfyNumbers: [Int] = playerNumbers.compactMap{ Int($0) } | ||
guard playerNumbers.count != count || range.intersection(veryfyNumbers).count != count else { return veryfyNumbers } | ||
print("입력이 잘못되었습니다.") | ||
return nil |
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.
옵셔널을 활용해서 예외상황을 처리해주었네요.
넘멋져용... 👍🏻
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.
감사합니다. 근데 코딩을 하면서 옵셔널을 너무 난무한게 아닌가? 라는 의문이 계속 들었습니다. 옵셔널 때문에 머릿속에서 정리가 안되는 부분도 있었습니다. 최대한 옵셔널 타입을 사용하지 않는 방향이 좋은 방법인가요?
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.
단순히 생각해보면 되지 않을까요?
값이 있을수도 있고 없을수도 있다면 옵셔널을...
그렇지 않다면 논옵셔널 타입으로 두면 되지 않을까... 싶습니다.
옵셔널을 제대로 알고 잘 활용한다면 난무하는게 큰 문제가 되진 않을 것 같아요.
@@ -17,9 +30,26 @@ func makeRandomNumber(to: Int, from: Int, count: Int) -> [Int]? { | |||
return nonOverlappingNumber | |||
} | |||
|
|||
func inputPlayNumber(from: Int, to: Int, count: Int) ->[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.
이 메소드도 반환타입이 옵셔널인 이유가 궁금합니다!
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.
랜덤한 난수를 뽑는 함수와 통일성을 주고자 했는데 반환타입을 일반 Int배열로 반환 해보는 방법도 시도 해보겠습니다.
목차 생성, Step1,Step2 파트 분리
같은 의미의 값이 반복적으로 사용되어 전역변수로 선언
README문서 현재 프로젝트 진행 상황에 맞게 수정
출력문 순서 수정
Style: inputPlayNumber함수 내부 playNumbers변수 개행 하여 수정
inputPlayNumber()함수를 inputPlayNumber()와 outputPlayNumber()로 분리 파라미터 from, to, count 삭제 전역변수 from: Int = 1, to: Int = 9 -> numberRange: ClosedRange<Int> = 1...9 수정
프로젝트 진행 상황에 맞게 README 수정
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.
수정사항 모두 확인했습니다!
요구사항도 모두 충족하고, 제가 말씀드려보았던 의견도 적극 반영해주셔서 감사합니다. 😁🙏🏻
추가 코멘트도 남겨보았는데, 당장은 수정하지 않아도 됩니다.
나중에 시간이 남는다면 개선해봅시다.
주말에 쉬셔야하는데.. STEP2 까지 달려와주시느라 고생하셨습니다. 🙌🏻
다음 프로젝트도 힘내세요! 🤗💪🏻
} while gameCount > Int.zero && playerScore.strikeScore != 3 | ||
} | ||
|
||
func makeRandomNumber() -> [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.
전역변수를 활용하기 때문에 파라미터를 없애준 것 같네요. 🤔
지금은 숫자게임에서만 쓰이는 함수긴 합니다만...
이 함수를 숫자게임 말고 다른 곳에서 사용하려면.. 내부 range의 값은 어떻게 정해줄 수 있을까요?
최대한 내부 코드를 건들지 않고, 추후에도 유연하게 활용할 수 있도록 파라미터를 적극 활용해주시면 좋습니다.
이유는 파라미터로 값을 주게 된다면 내부코드를 건들지 않고
파라미터의 값을 바꿔서 할당해주는 것만으로도 다른 용도로 유용하게 쓸 수 있기 때문입니다.
이 부분을 당장 수정은 하지 않아도 됩니다.
이 후 다음 프로젝트에서는 꼭 고민하셔서
수정될 수도 있는 값이 있다면, 파라미터도 적극적으로 활용해보셨으면 하는 마음에 코멘트 남겨봅니다 😊
|
||
import Foundation | ||
|
||
func makeRandomNumber(to: Int, from: Int, count: Int) -> [Int]? { | ||
let numberCount: Int = 3 | ||
let numberRange: ClosedRange<Int> = 1...9 |
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.
음.. 어떤 숫자 카운트이고, 어떤 숫자범위죠..?
이름이 명확하지 않은 것 같네요 🤔
printInputError() | ||
return inputPlayNumber() | ||
} | ||
return inputPlayNumbers |
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.
오타가 있는 것 같습니다!
outputPlayNumber()
checkPlayNumber()
inputPlayNumber()
세개의 함수 이름이 PlayerNumber
로 적어주시려고 했던 것 같은데... 맞죠!?
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 와 case내부를 한줄로 작성 -> 내려쓰기 하여 가독성 case내부가 눈에 잘들어오도록 수정 | ||
- outputPlayNumber함수 [Int]?을 [Int]로 변경 | ||
- checkPlayNumber 함수에서 유효성 검사를 완료하고, 반환하는 값에 대해선 nil이 아닌 것이 확정이기에 일반 타입으로 수정 |
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.
와... 리드미 꼼꼼히 적어주셨네요.
정성 대박 👍🏻
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.
아리 리드미를 많이 참고했습니다! 리드미 앞으로도 꾸준히 잘 작성해보겠습니다!! :)
@leeari95
Ari! 시간이 많이 늦었습니다 ㅜㅜ 죄송합니다!
진행 상황
첫 번째 고민했던 부분
사용자 메뉴를 출력하고 메뉴를 입력받는 함수 (게임시작, 게임종료) 를 구현하기위해 selectGameMenu() 을 만들었습니다.
잘못된 입력을 넣게되면 입력이 잘못되었다는 문구를 출력하고 반복문을 다시 실행할수 있게 하고 싶어 고민했습니다.
해결방법
반복문을 다시 실행할수 있게 하고싶어 반복문 조건을 true 로 설정하였습니다.
두 번째 고민했던 부분
입력 받은 숫자 배열을 조건에 맞게 유효성을 검증하는 과정에서 한번에 반복문 없이 해결하는 부분에 대해 고민하였습니다.
해결 방법
Set타입에 from...to로 범위를 지정하고 입력받은 배열과 교집합을 통해 수가 1...9 안에 있는지와 중복을 한번에 해결할 수 있었습니다
피드백 받고싶은 부분
첫번째 고민했던 부분에서
Step1에서도 이러한 방식으로 반복문 조건을 넣어주었는데 언제까지 반복하는지 한눈에 이해할수없어 가독성이 떨어지는 코드가 되었었습니다.
사용자 메뉴 를 출력 실행하는 함수에서는 함수를 종료하는 조건을 입력받기 때문에
종료조건이 아니라면 게속 반복되어도 괜찮지 않을까? 생각하여 반복문 조건을 true 로 설정해두었습니다!
조건을 다른방식으로 넣어주는게 좋았을지 의문이들어 조언을 듣고싶습니다!