Skip to content
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

Open
wants to merge 3 commits into
base: ss_20_Reimos7
Choose a base branch
from

Conversation

Reimos7
Copy link

@Reimos7 Reimos7 commented Sep 22, 2024

@jaemuYeo

고민했던 점

주제

아이스크림 스틱 부분의 공백 부분을 변수로 넣어서 작성 했는데 이 방법이 맞는건지 고민했습니다.

조언을 얻고 싶은 부분

주제

  • 아이스크림 스틱 부분에서 더 좋은 방법이 있는지 궁금합니다.

Copy link

@jaemuYeo jaemuYeo left a 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("")

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){

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){

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) | | ")

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)

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{

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{

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 {

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){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수명과 매개변수 명이 겹치고 있군요!
위 코멘트를 참고하여 함수명을 수정해보면 좋을 것 같아요 :)


}

func stick(_ stickLength: Int){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 함수도 위 코멘트를 참고하여 수정해주세요! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants