Skip to content

[산타] 버디 미션 제출합니다.#15

Open
stopmin wants to merge 2 commits intostopmin-mainfrom
stopmin/christmas
Open

[산타] 버디 미션 제출합니다.#15
stopmin wants to merge 2 commits intostopmin-mainfrom
stopmin/christmas

Conversation

@stopmin
Copy link
Collaborator

@stopmin stopmin commented Feb 26, 2024

구현하면서 고민되었던 점

  • 클래스 나누는게 좀 생각보다 고민이 들었습니다.

자랑하고 싶은 점

  • 코틀린을 어느정도 활용하려고 시도했던 것 같다는 점인거같습니다.

죄송한점

  • 테스트코드 아직 못짰습니다 ㅠ
  • 예외처리 아직 일부 구현이 안되었습니다!! 테스트코드는 일단 다 통과합니닷..
  • 마저 화,수동안 구현하겠습니다!!

나름의 변명...

어.. 제가 주말에 맥북을.. 온천천에서 떨어뜨려서 멘탈이 조금 바사사삭...됐습니다.. 하핫
미안합니닷..!
리뷰 하고싶은데 코드가 안올라와서 속상하실까봐 미리 pr 올려둡니닷..!

@stopmin stopmin requested review from 1jeongg and gusah009 February 26, 2024 16:56
@stopmin stopmin self-assigned this Feb 26, 2024
Copy link
Collaborator

@gusah009 gusah009 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~~ 👍👍

TODO("프로그램 구현")
val eventPlanner = EventPlanner()
val restaurant = Restaurant(eventPlanner)
restaurant.open()
Copy link
Collaborator

Choose a reason for hiding this comment

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

변수명이나 메서드명이 스토리를 읽는 것 같아서 재밌네요 👍

Comment on lines +93 to +95
init {
eventPlanner.greeting()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

인삿말만 init으로 뺀 이유가 있을까요? 없다면 open() 메서드에 포함시켜도 좋을 것 같습니다

Comment on lines +17 to +32
fun checkChristmasDiscount(date: Int): DiscountResult {
val christmasDiscount = Discount.CHRISTMAS_DISCOUNT.amount + (date - 1) * 100
return DiscountResult(Discount.CHRISTMAS_DISCOUNT, christmasDiscount)
}

fun checkWeekDiscount(date: Int, order: List<MenuItem>): DiscountResult? {
val day = Date.getDayOfWeek(date)?.getDateType()
?: return null

val discountAmount = when (day) {
DateType.WEEKDAY -> Discount.WEEKDAY_DISCOUNT.amount * order.filter { it is MenuItem.Dessert }.size
DateType.WEEKEND -> Discount.WEEKDAY_DISCOUNT.amount * order.filter { it is MenuItem.Main }.size
}

return DiscountResult(discountCode = Discount.WEEKDAY_DISCOUNT, discountAmount = discountAmount, any = day)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

각 메서드가 enum 각각의 원소와 관련이 있어 보입니다, Discount enum 클래스에 abstract로 녹여낼 수 있을 것 같아요.

Comment on lines +56 to +58
fun findByAmount(amount: Int): RewardBadge? {
return values().lastOrNull { amount >= it.threshold }
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

enum 원소의 순서에 크게 의존하고 있는 것 같아요. 주석으로 주의를 주거나 해당 방식은 지양하는게 좋을 것 같습니다.

참고: https://stackoverflow.com/questions/3820149/enum-values-is-an-order-of-returned-enums-deterministic


companion object {
val event = Event()
val items = Menu.items
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 items를 재정의하신 이유가 있을까요?

println("안녕하세요! 우테코 식당 12월 이벤트 플래너입니다.")
}

fun receiveVisitDate(): Int {
Copy link
Collaborator

Choose a reason for hiding this comment

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

date는 날짜 전체를 의미하는 것 같아서 dayOfMonth 같은 용어도 괜찮아 보여요

Comment on lines +19 to +26
// private fun getMenuItems(): List<MenuItem> {
// return eventPlanner.takeOrder().flatMap { orderItem ->
// val itemName = orderItem[0]
// val quantity = orderItem[1].toInt()
//
// List(quantity) { items.find { item -> item.name == itemName }!! }
// }
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

주석은 지워도 될 것 같아요~

Copy link
Collaborator

@1jeongg 1jeongg left a comment

Choose a reason for hiding this comment

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

이번주도 고생 많으셨어요~!! 메뉴가 주말/주중 구분에서 깔끔하게 카테고리별로 분류하신 점이 인상깊었어요! 👍👍

맥북.. 제가 다 마음이 아프네요 ㅠㅠ

다음주도 화이팅입니다 🔥🔥

class Drink(name: String, price: Int) : MenuItem(name, price)
}

object Menu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 이런식으로 사용하니까 카테고리별로 보기 쉽네요! 👍👍

Comment on lines +27 to +50
fun getMenuItems(): List<MenuItem> {
while (true) {
val orderItems = eventPlanner.takeOrder()

if (orderItems.distinctBy { it[0] }.size < orderItems.size) {
println("[ERROR] 유효하지 않은 주문입니다. 다시 입력해 주세요.")
continue
}

val menuItems = orderItems.flatMap { orderItem ->
val itemName = orderItem[0]
val quantity = orderItem.getOrNull(1)?.toIntOrNull()

if (quantity == null || quantity < 1 || items.none { it.name == itemName }) {
println("[ERROR] 유효하지 않은 주문입니다. 다시 입력해 주세요.")
return@getMenuItems listOf<MenuItem>()
}

List(quantity) { items.find { item -> item.name == itemName }!! }
}

return menuItems
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이번 요구사항에 함수는 15라인 이하라고 되어있어서 분리하는게 좋을 것 같아요! 그리고 잘못된 값을 입력하면 IllegalArgumentException을 발생시켜야된다고도 나와있어요!

}

fun receiveVisitDate(): Int {
println("12월 중 식당 예상 방문 날짜는 언제인가요? (숫자만 입력해 주세요!)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 선택적일 수 있긴 하지만 이벤트 목표에 12월 이벤트 참여 고객의 5%가 내년 1월 새해 이벤트에 재참여하는 것 이 있어서 몇 월인지 계속 바뀔 수 있으니까 12월도 MAGIC_NUMBER로 빼두면 어떨까요?

class Event {
enum class Discount(val description: String, val amount: Int) {
CHRISTMAS_DISCOUNT("크리스마스 디데이 할인", 1000),
WEEKDAY_DISCOUNT("평일 할인", 2023),
Copy link
Collaborator

Choose a reason for hiding this comment

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

주말 할인은 일부러 빼신 건가요??

totalPrice: Int
): List<DiscountResult> {
if (totalPrice <= 10_000) {
return listOf()
Copy link
Collaborator

Choose a reason for hiding this comment

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

빈 리스트를 반환하고 싶다면 가독성을 위해 emptyList()는 어떨까요?

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.

3 participants