-
Notifications
You must be signed in to change notification settings - Fork 221
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
[Round2] Step3 - kingkiboots #758
[Round2] Step3 - kingkiboots #758
Conversation
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.
스텝3로 완벽하게 잘 구현해주셨군요!!
몇 가지 의견을 남겼으니 확인해주세요 :)
@@ -6,3 +6,34 @@ | |||
// | |||
|
|||
import Foundation | |||
|
|||
let combineIndexWithLottoHistorySuffix = { (index: Int) -> String in |
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.
클로저를 활용해보셨군요!
상수에 클로저를 저장해서 사용하는 것과 함수로 만들어 사용하는 것을 고민하셨던 것 같아요.
각각의 장단점이 있습니다.
내부적으로 로직이 돌아가고 결과를 반환하는 형식은 크게 다를 것이 없습니다.
피드백 세션 내용에도 있듯 '함수는 이름이 있는 클로저' 이니까요!
다면 위의 문장에서 말한 것처럼 '함수는 이름이 있다'라고 표현되었는데
지금 처럼 상수에 저장한다고 해서 저 클로저에 이름이 붙는 것은 아니예요.
저런 동작을 하는 클로저를 특정 상수에 '할당' 한 것 뿐입니다.
그래서 위에 상수에 저장한 방식과 함수를 구현하는 방식을 비교하는 것은 서로 다른 것을 비교하는 것과 같다고 볼 수 있을 것 같아요.
클로저 자체와 함수를 비교한다면 할 수 있겠지만 현재는 구현 방식이 다르다라고 생각합니다!
만약 함수와 비교한다면
func 예시함수() {
}
let combineIndexWithLottoHistorySuffix = 예시함수()
이 표현이 맞을 것 같아요!
아무래도 함수를 구현하는 것이 재사용성과 명확한 네이밍을 통해 가독성을 높이기에는 더 좋은 방법이지 않을까 생각합니다 :)
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 "\(index)\(lottoHistorySuffix)" | ||
} | ||
|
||
func generateLottoHistories() -> Dictionary<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.
반환을하는 딕셔너리 타입에서 키값은 몇 회차인지를 저장하고있는데
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.
네 그럼 그렇게 수정하겠습니다!
return lottoHistories | ||
} | ||
|
||
func findLottoHistory(at index: 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.
매개변수
몇 회차인지에 대해 전달인자를 통해 주입받는데 index라는 네이밍 보다 round가 조금 더 어울리지 않을까 생각합니다!
킹키부츠님은 어떻게 생각하시나요~?
네이밍
만약 위의 매개변수가 round로 바뀐다면 find라는 '찾다'의 의미보다 '검색(serch)' 등의 다른 표현으로도 바꿔볼 수 있을 것 같아요.
history는 없애도 될 것 같습니다! -> ex. 몇 회차 로또 번호 검색
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.
색인인 index보다 round 라는 단어가 회차라는 그 역할에 더 부합한 것 같습니다!
또한 find 보다는 무엇을 검색해서 정보를 얻는다 라는 느낌이라서 확 와닿네요!
안녕하세요!
|
코드도 훨씬 간결해지고 네이밍들이 명확해진 것 같습니다! 👍 |
@jaemuYeo
안녕하세요! Round2 Step3 Pull Request 작성하여 요청드립니다!
고민했던 점
for in 이 아닌 while
Dictionary에 들어갈 key를 만드는 함수
클로저 사용해보기
조언을 얻고 싶은 부분
함수 vs 클로저