-
Notifications
You must be signed in to change notification settings - Fork 253
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
[Round1] Step2 - Reimos7 #1003
base: ss_20_Reimos7
Are you sure you want to change the base?
[Round1] Step2 - Reimos7 #1003
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.
프로젝트 하시느라 고생많으셨습니다 👏
앗 제목은 스텝2인데 스텝3의 코드도 포함되어있군요?!
일단은 진도가 조금 늦은감이 있어 한번에 리뷰드리겠습니다!
Round2에서는 꼭 하나의 스텝씩 PR 보내주세요 :)
스텝이 이어지기때문에 다음 스텝을 미리 push하게되면 코드가 꼬일수도 있어요!
순서도
현재 순서도가 공유되어있지않아요 ㅠㅠ
순서도도 첨부하여 재요청시 함께 공유해주세요!
파일 분리
스텝3는 파일이 분리되어있는데
아이스크림 그리기인 스텝2는 main.swift파일에 구현되어있네요!
스텝별로 분리하여 관리하면 좋습니다 👍
함수의 호출만 main파일에서 진행해주세요!
(아직 Merge 되기 전이니 다음 라운드로 가지않고 수정 후 Merge되면 넘어가주세요!)
for _ in 1...10 { | ||
print("*", terminator: "") | ||
} | ||
print("") |
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.
해당 print는 개행을 위해 호출한 건가요~?
\n 을 통해서도 개행이 가능하답니다!
함수의 호출 또한 메모리를 잡아먹는 요소가 될 수 있기에 최소화하는게 좋아요!
|
||
func iceCream(size: 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.
API 디자인 가이드라인에 함수의 네이밍 앞쪽에는 동사로 표현해주라고 권장하고 있습니다!
학습 기간 중에 꼭 디자인 가이드라인의 네이밍 부분을 정독해보시길 추천드려요 💪
예를 들어 makeIceCreamBody, printIceCreamBody와 같이요.
아래에 스틱을 그릴때는 iceCreamStick이라고 명시되어있는데 이 곳에는 몸통을 표현하고 있지 않아 그 부분도 명확하게 해주면 좋습니다 :)
- 중괄호 앞쪽에 띄어쓰기가 되어있지 않아요!
스위프트의 모든 중괄호는 앞쪽에 띄어쓰기가 됩니다! (아래 함수도 참고해주세요)
} | ||
} | ||
|
||
func iceCreamStick(size: 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.
이 함수의 네이밍도 위에 코멘트를 참고하여 수정해보면 좋을 것 같아요 :)
|
||
func iceCreamStick(size: Int){ | ||
for _ in 1...size { | ||
print("\(empty) | | ") |
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.
empty 변수를 넣은 이유가 궁금합니다!
- 막대기 뒤의 공백은 없어도 될 것 같아요!
print("") | ||
} | ||
|
||
iceCream(size: 8) |
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 bodyLength(_ bodylength: Int){ | ||
for _ in 1...bodylength { | ||
for _ in 1...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.
바깥 for문과 띄어쓰기 컨벤션이 다릅니다!
|
||
func bodyLength(_ bodylength: Int){ | ||
for _ in 1...bodylength { | ||
for _ in 1...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...3은 무엇을 의미하나요?
|
||
func iceCream(size: Int){ | ||
for _ in 1...size { | ||
for _ in 1...10 { |
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...10은 무엇을 의미하나요?
print("") | ||
|
||
|
||
func bodyLength(_ bodylength: 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.
함수명과 매개변수 명이 겹치고 있군요!
위 코멘트를 참고하여 함수명을 수정해보면 좋을 것 같아요 :)
|
||
} | ||
|
||
func stick(_ stickLength: 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.
해당 함수도 위 코멘트를 참고하여 수정해주세요! 💪
@jaemuYeo
고민했던 점
주제
아이스크림 스틱 부분의 공백 부분을 변수로 넣어서 작성 했는데 이 방법이 맞는건지 고민했습니다.
조언을 얻고 싶은 부분
주제