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

JuiceMaker Step1 - lina #1

Closed
wants to merge 14 commits into from
Closed

JuiceMaker Step1 - lina #1

wants to merge 14 commits into from

Conversation

lina0322
Copy link

고민했던 부분

1. 쥬스 메이커를 하나만 만들고, 매개 변수로 과일과 수량 등의 레시피를 받아서 구현 vs 지금처럼 각각의 주스마다 함수를 만드는 방법으로 구현

-> 전자로 구현할 경우 새로운 레시피가 들어왔을때 수정이 편할 것 같아서 구현을 시도하다가,
각 쥬스마다 따로 버튼이 나눠져있어서 후자로 구현했습니다.
지금처럼 전부다 버튼별로 나눠저 있는게 나을지...
아니면 쥬스메이커, 재고확인하는 함수 2개 정도만 구현해서
매개변수로 레시피나, 과일 종류를 받아서 작동하는게 좋을지....!!!
어떤 쪽이 좋을지 몰라서 조언을 구합니다!!🙈

2. 읽기 전용으로 과일 재고를 확인하기

-> 읽기 전용 자체를 처음 사용해보아서 이렇게 사용하는게 맞는지 잘 모르겠습니다...!
읽기 전용으로 과일마다 각각 stock 변수를 따로 구현하고,
실제 클래스 내에서 사용되는 변수는 위에 따로 선언하고 각각 private를 붙여
밖에서는 접근할 수 없게(?) 나름 구현했는데.. 맞는 방식인지 확인 부탁드립니다!🙉

3. Alert

-> 지금은 JuiceMaker부분만 가지고 하기위해
재고가 모자르거나, 쥬스가 완성된걸 저만 확인하려고 print로 구현해놓긴했는데,
이 부분을 Alert로 구현한다면 지금 함수는 리턴을 Bool로 해서
ViewController에서 작업하는게 더 나을 것 같은데, 이 부분도 조언을 구합니다..!🙊

Copy link
Author

@lina0322 lina0322 left a comment

Choose a reason for hiding this comment

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

앗, 댓글 달고있는데 사라졌어요!
방금 말씀해주신 부분 저의 실수 + 연달은 복붙에 의한 결과입니다. 수정하겠습니다~
감사합니다..!

Copy link

@daheenallwhite daheenallwhite left a comment

Choose a reason for hiding this comment

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

명확한 method naming 이 돋보이네요!
덕분에 한눈에 각 method 가 어떤 역할을 맡는지 파악할 수 있었어요.

코멘트 단 부분과 질문에 대한 답변도 아래에서 확인해주세요.

  1. 쥬스 메이커를 하나만 만들고, 매개 변수로 과일과 수량 등의 레시피를 받아서 구현 vs 지금처럼 각각의 주스마다 함수를 만드는 방법으로 구현

-> 전자로 구현할 경우 새로운 레시피가 들어왔을때 수정이 편할 것 같아서 구현을 시도하다가,
각 쥬스마다 따로 버튼이 나눠져있어서 후자로 구현했습니다.
지금처럼 전부다 버튼별로 나눠저 있는게 나을지...
아니면 쥬스메이커, 재고확인하는 함수 2개 정도만 구현해서
매개변수로 레시피나, 과일 종류를 받아서 작동하는게 좋을지....!!!
어떤 쪽이 좋을지 몰라서 조언을 구합니다!!🙈

두 방법 각각 특징이 있는데요
첫번째 방법을 사용한다면 레시피를 담당하고 외부에서 쥬스메이커에게 이를 넘겨줄 오브젝트가 필요하겠죠? 역할을 좀더 세분화할 수 있는 방법으로 보여요.
두번째 방법을 사용한다면 첫번째 방법처럼 외부에서 담당할 애는 없지만 내부에서 주스마다 함수가 필요하겠죠? 각 메소드가 각 주스를 제조하는 역할로 잘 분리될 것 같아요.

위의 의견은 제 주관적인 생각이고 lina 생각엔 각 방법을 사용할 경우 장/단점엔 뭐가 있을까요?
확장성을 생각한다면 어떤 점을 고려하는게 더 좋을지도 고려해보면 더 적절한 방법이 나오지 않을까요?

  1. 읽기 전용으로 과일 재고를 확인하기

-> 읽기 전용 자체를 처음 사용해보아서 이렇게 사용하는게 맞는지 잘 모르겠습니다...!
읽기 전용으로 과일마다 각각 stock 변수를 따로 구현하고,
실제 클래스 내에서 사용되는 변수는 위에 따로 선언하고 각각 private를 붙여
밖에서는 접근할 수 없게(?) 나름 구현했는데.. 맞는 방식인지 확인 부탁드립니다!🙉

소프트웨어에는 맞고 틀리다는 없어요☺️ 상황에 따라 좀더 적절하고 덜 적절한(?) 방식이 있을 수는 잇죠!
간단한 몇가지 메뉴를 만들 수 있는 쥬스메이커라면 지금 방식도 좋겠지만
만약에 쥬스메이커 앱이 좀 더 커져서 더 다양한 음료를 만들어야 된다면 어떻게 하는게 좋을까요?
관리할 stock 이 많아지고 재료가 많아진다면, 지금 방식대로면 그만큼 property 가 늘어나겠죠?
stock 을 담당하는 클래스나 구조체를 만들어도 좋은 해결방식이 될 것 같아요!
접근 제어자 관련해서는 코멘트단 곳의 사이트 설명을 한번 읽어보세요

  1. Alert

-> 지금은 JuiceMaker부분만 가지고 하기위해
재고가 모자르거나, 쥬스가 완성된걸 저만 확인하려고 print로 구현해놓긴했는데,
이 부분을 Alert로 구현한다면 지금 함수는 리턴을 Bool로 해서
ViewController에서 작업하는게 더 나을 것 같은데, 이 부분도 조언을 구합니다..!🙊

나중에 ViewController 에서 받을 유저 인풋값을 쥬스메이커에서 고려하는건 단일책임에 위배되는 걸로 보여요
쥬스 메이커는 말그대로 주스를 만드는 일만 하는게 맞지 않을까요?
그런 의미에서 안내문을 print 하는 함수가 쥬스메이커에 있는게 적절한 역할을 했다고 볼 수 있을까요?

Comment on lines 14 to 50
private var strawberry: Int = 10
private var banana: Int = 10
private var pineapple: Int = 10
private var kiwi: Int = 10
private var mango: Int = 10


// MARK : 읽기 전용 과일 재고
var strawberryStock: Int {
get {
return strawberry
}
}

var bananaStock: Int {
get {
return banana
}
}

var pineappleyStock: Int {
get {
return pineapple
}
}

var kiwiStock: Int {
get {
return kiwi
}
}

var mangoStock: Int {
get {
return mango
}
}

Choose a reason for hiding this comment

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

여기서 access 만 가능한 ##stock property 를 사용했군요
Swift 의 다양한 제어 접근자를 찾아보는 건 어떨까요?
https://docs.swift.org/swift-book/LanguageGuide/AccessControl.html#ID17

Comment on lines 105 to 123
func AddStrawberryStock() {
strawberry = strawberry + 1
}

func AddBananaStock() {
banana = banana + 1
}

func AddPineappleStock() {
pineapple = pineapple + 1
}

func AddKiwiStock() {
kiwi = kiwi + 1
}

func AddMangoyStock() {
mango = mango + 1
}

Choose a reason for hiding this comment

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

여기서는 다른 method naming 과 다르게 대문자로 Method 이름이 시작하는데 다르게 한 이유가 있을까요?

Comment on lines 52 to 59
// MARK : alert 구현 전, 확인용 메세지
func showOutOfStock() {
print("재료가 모자라요. 재고를 수정할까요?")
}

func showSuccess() {
print("쥬스 나왔습니다! 맛있게 드세요!")
}

Choose a reason for hiding this comment

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

바뀌지 않는 안내용 문자열이라면 항상 새로 ""로 시작하는게 좋을까요?

Comment on lines 63 to 66
if kiwi >= 3 {
kiwi = kiwi - 3
return showSuccess()
}

Choose a reason for hiding this comment

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

Swift 에는 if 도 있지만 guard 라는 것도 있는데 어떤 차이가 있을까요?

@lina0322
Copy link
Author

수정중입니다! 리뷰 다시 필요할때 태그하겠습니다~
그 때 앞서 해주신 리뷰에 대한 답도 달겠습니다!😄

@lina0322 lina0322 closed this Nov 18, 2020
hcooch2ch3 referenced this pull request in hcooch2ch3/ios-juice-maker Nov 19, 2020
daheenallwhite pushed a commit that referenced this pull request Nov 19, 2020
add(JuiceMaker.swift / jake) : step1 쥬스메이커 타입 정의
kdaramz added a commit to kdaramz/ios-juice-maker that referenced this pull request Nov 22, 2020
yagom pushed a commit that referenced this pull request Nov 23, 2020
Step1 쥬스 메이커 타입 정의
yagom pushed a commit that referenced this pull request Nov 23, 2020
add(step1 / kkomal): step1 구현 완료
boseongv pushed a commit that referenced this pull request Mar 9, 2021
delmaSong pushed a commit that referenced this pull request Mar 11, 2021
seizze pushed a commit that referenced this pull request Mar 16, 2021
steven-test 브런치에서 머지
seizze pushed a commit that referenced this pull request Mar 16, 2021
[STEP01] feat: implement JuiceMaker, FruitStockManager, JuiceRecipe
corykim0829 pushed a commit that referenced this pull request Mar 17, 2021
corykim0829 pushed a commit that referenced this pull request Mar 17, 2021
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
- 연관값을 사용하여 상황에 맞는 에러메세지를 출력하기 위해 수정
- rawValue삭제 후 description 로직 수정
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
- xcode의 오류로 인한 종료의 문제가 발생하여 언어 변경
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
- 재고랑과 필요수량이 같은 경우를 누락하여 등호 추가
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
GREENOVER pushed a commit that referenced this pull request Oct 23, 2021
- 프로젝트 진행 시 다른 파일에서 값을 불러올 것이라 예상하여 접근제어 수정
GREENOVER pushed a commit that referenced this pull request Feb 16, 2022
GREENOVER pushed a commit that referenced this pull request Feb 16, 2022
GREENOVER pushed a commit that referenced this pull request Feb 16, 2022
쥬스 메이커 [STEP 1] 헨리
zdodev pushed a commit that referenced this pull request Feb 17, 2022
Monsteel pushed a commit that referenced this pull request Feb 18, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
- FruitStore checkInventory 오류 수정
- FruitStore calculateUsableInventory 함수 분리
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
- Unit test 통과
- FruitStoreError Error 프로토콜 제거
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
- [FruitType: Int] -> FruitInventory 전환.
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
wonhee009 pushed a commit that referenced this pull request Feb 19, 2022
- FruitStore 메서드들 매개변수명 수정
Monsteel pushed a commit that referenced this pull request Sep 6, 2022
JoSH0318 pushed a commit that referenced this pull request May 19, 2023
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.

None yet

2 participants