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

[Round3] Step1 - hansol #342

Open
wants to merge 2 commits into
base: ss_19_hansol
Choose a base branch
from

Conversation

enqnsol614
Copy link

@enqnsol614 enqnsol614 commented Sep 6, 2024

@jaemuYeo
항상 상세히 리뷰해주셔서 감사합니다. 오늘은 Round3에 step1 PR 요청드립니다!
오늘이 마지막 리뷰라고 하셔서 어쩔 수 없이 Step2까지 함꼐 커밋하였습니다.

고민했던 점

  • 커피 주문 시스템에서 메뉴를 Dictionary로 관리하고, 잔액을 확인한 후 커피를 구매하는 방식을 시도했습니다.
  • 저도 모르게 makeCoffe(coffee....) 이런식으로 함수를 만들어 함수명이 매개변수와 중복되어 2번째 커밋에 order 와 make로 수정하여 올렸습니다.

해결되지 않은 점

  • 어느 변수에 Optional 처리해야하는 지가 항상 난해합니다..

조언을 얻고 싶은 부분

  • enum 대신 그냥 정수나 문자열로 메뉴를 관리하는 것이 더 나을까요? enum 이라는 열거형을 실제 많이 사용하나요??

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.

마지막 PR도 너무 고생많으셨습니다!
5주동안 저희 과정을 통해 많은 도움이 되셨길 바래요 🙏
마지막으로 몇 가지 코멘트와 함께 마무리하겠습니다!

커밋 메시지

현재 많은 타입과 메서드들이 구현되었는데 커밋 메시지가 두 개뿐이군요!
어떤 경우마다 커밋 메시지를 남기면 좋을까요??

파일 분리

현재 CoffeeShop이라는 파일 하나에 모든 타입이 전부 들어가있어요!
타입별로 또는 관련 타입별로 묶어서 파일 분리를 해주면 좋습니다 💪

옵셔널 처리

옵셔널 타입을 사용하는 곳이라면 어떤 곳이든 바인딩 처리를 해주어야 합니다! 🙌

열거형 사용

물론 이번 Round에서 열거형을 경험하고자 넣은 부분도 있긴하지만
다른 방안도 있습니다! 코드에 정답은 없으니까요 :)
열거형은 실제로도 엄청엄청 많이 사용됩니다!!
열거형을 사용하면 어떤 이점이 있는지 왜 이런 언어적 기능이 탄생했는지에 대해
조사해보면 좋을 것 같아요 :)

이니셜라이저

현재 두개의 클래스에서 모두 이니셜라이저 없이 변수에 기본값을 주거나 옵셔널 타입으로 해주었는데
이니셜라이저를 잘 활용하면 인스턴스 생성시 많은 이점이 있습니다!
이후 학습하며 꼭 적용해보면 좋을 것 같아요!

case cafeLatte = "카페라떼"
case cappuccino = "카푸치노"

var orderMenu: String {
Copy link

Choose a reason for hiding this comment

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

orderMenu라는 네이밍이 변수보다 함수와 가까운 네이밍인 것 같아요!
간단하게 name 정도로 해줘도 좋을 것 같습니다 :)
외부에서 사용할때 Coffee.name으로 하니깐요 😀

}

class Person {
var name: String = ""
Copy link

Choose a reason for hiding this comment

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

init을 활용하지 않고 기본값을 준 이유가 궁금해요!

}
}

class Person {
Copy link

Choose a reason for hiding this comment

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

Person타입은 왜 클래스로 선언하였는지 궁금합니다!

var name: String = ""
var money: Int = 0

func order(_ coffee: Coffee, of coffeeShop: CoffeeShop, by name: String) {
Copy link

Choose a reason for hiding this comment

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

인자 레이블과 파라미터의 활용 너무 좋습니다! 👍

var money: Int = 0

func order(_ coffee: Coffee, of coffeeShop: CoffeeShop, by name: String) {
if let price = coffeeShop.menu[coffee] {
Copy link

Choose a reason for hiding this comment

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

if문 다음의 로직 흐름으로 이어질 필요가 없다면 guard-let을 활용하는게 좋답니다 :)
저는 개인적으로 guard문을 통과하지 못할때 빠르게 함수가 종료된다는 것과 가독성때문에
guard문을 조금 더 애용하고 있어요!

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