[럿고] 치킨집 미션 제출합니다. #5

ksy90101 · 2020-06-27T12:54:27Z

와.. 진심 너무 어렵네요 ㅠㅠ
웹 처럼 하려다가 완전 망한거 같아요...
마음껏 까주시면 감사하겠습니다!

…2020 into tdd-rutgo-pr

KS-KIM

안녕하세요 럿고.
치킨집 미션 잘 구현해 주셨네요.
간단한 피드백 남겼으니 궁금한 점 있으시면 DM 남겨주세요.

KS-KIM · 2020-06-27T15:00:10Z

src/main/java/controller/PaymentController.java

+	private int getTableNumber() {
+		OutputView.printTables(tableService.findTables());
+		return InputView.inputTableNumber();
+	}


OrderController에 동일한 메서드가 있는데, 재활용 할 수는 없을까요?

웹으로 하려고 하다보니, 이렇게 된거 같아요.ㅠㅠ
웹이였다면 인자로 받아오기만 하면 되기때문에 중복 코드가 발생하지 않았을꺼 같은데...ㅠㅠ
이건 개인적으로 이렇게 남겨두고 싶어요!

KS-KIM · 2020-06-27T15:02:23Z

src/main/java/domain/Function.java

+	ORDER(1, new OrderController(new MenuService(new MenuRepository()),
+		new TableService(new TableRepository(), new MenuRepository()))),
+	PAYMENT(2, new PaymentController(new TableService(new TableRepository(), new MenuRepository()))),


매번 새로운 TableRepository, MenuRepository 인스턴스를 생성할 필요가 있을까요?

KS-KIM · 2020-06-27T15:05:47Z

src/main/java/controller/PaymentController.java

+			Payment payment = Payment.of(paymentNumber);
+			BigDecimal account = payment.pay(table.getOrderHistories());


결제 타입을 식별하고, 테이블에 있는 모든 주문의 가격을 계산하여 지불할 금액을 계산하는 것은 TableService가 가져야 할 역할이 아닐까요? 컨트롤러는 입출력 흐름을 제어하고 서비스에 비즈니스 로직에 대한 수행을 위임하는 것으로 충분한 역할을 하는 것이라고 생각하는데, 럿고는 어떻게 생각하시는지 궁금하네요.

그렇네요... 제가 급하게 하다 보니...ㅎㅎ 변경했습니다!

KS-KIM · 2020-06-27T15:10:25Z

src/main/java/domain/menu/MenuRepository.java

+	public Menu findById(final int number) {
+		return menus.stream()
+			.filter(menu -> menu.isSameNumber(number))
+			.findFirst()
+			.orElseThrow(() -> new IllegalArgumentException("해당 메뉴가 존재하지 않습니다. number = " + number));
+	}


존재하지 않을 수 있는 값이니 Optional 그 자체를 반환해도 될 것 같아요.

KS-KIM · 2020-06-27T15:12:04Z

src/main/java/domain/payment/CardPayment.java

+		return new BigDecimal(String.valueOf(chickenPaymentAmount));
+	}
+
+	private Optional<DisCountStrategy> findDisCountStrategy(Class<? extends DisCountStrategy> disCountStrategy) {


Suggested change

private Optional<DisCountStrategy> findDisCountStrategy(Class<? extends DisCountStrategy> disCountStrategy) {

private Optional<DisCountStrategy> findDiscountStrategy(Class<? extends DisCountStrategy> disCountStrategy) {

KS-KIM · 2020-06-27T15:25:32Z

src/test/java/domain/table/TableTest.java

+
+	@BeforeEach
+	void setUp() {
+		table = new Table(1, new ArrayList<>());


빈 리스트를 인자로 넘겨 줄 것이라면, 테이블 번호만 받아서 빈 주문 리스트를 내부적으로 생성해주는 생성자를 가져도 되지 않을까요?

KS-KIM · 2020-06-27T15:29:21Z

src/test/java/domain/table/TableTest.java

+	@DisplayName("테이블에 주문내역이 제대로 들어가는지 확인하는 테스트")
+	@Test
+	void addOrderHistoryTest() {
+		MenuRepository menuRepository = new MenuRepository();


도메인 테스트인데 persistence layer에 속하는 MenuRepository 클래스가 개입해도 괜찮을까요?
Menu를 제공할 수 있는 Fixture를 만드는 것도 하나의 방법일 것 같아요.

그렇네요

KS-KIM · 2020-06-27T15:30:29Z

src/test/java/domain/table/OrderHistoryTest.java

+import domain.menu.MenuRepository;
+
+public class OrderHistoryTest {
+	private final MenuRepository menuRepository = new MenuRepository();


도메인 테스트인데 persistence layer에 속하는 MenuRepository 클래스가 개입해도 괜찮을까요?
Menu를 제공할 수 있는 Fixture를 만드는 것도 하나의 방법일 것 같아요.

그렇네용

KS-KIM · 2020-06-27T15:31:03Z

src/test/java/domain/table/OrderHistoriesTest.java

+
+class OrderHistoriesTest {
+
+	private final MenuRepository menuRepository = new MenuRepository();


도메인 테스트인데 persistence layer에 속하는 MenuRepository 클래스가 개입해도 괜찮을까요?
Menu를 제공할 수 있는 Fixture를 만드는 것도 하나의 방법일 것 같아요.

그렇네용

KS-KIM · 2020-06-27T15:34:44Z

src/test/java/domain/menu/CategoryTest.java

+
+	@DisplayName("제대로 출력되는지 확인하는 테스트")
+	@ParameterizedTest
+	@CsvSource(value = {"CHICKEN:[치킨]", "BEVERAGE:[음료]"}, delimiter = ':')


delimiter를 따로 지정하신 특별한 이유가 있나요?
기본 delimiter인 comma(',')를 사용해도 테스트하는데 문제가 없을 것 같거든요.
취향이라면 존중합니다😊

취향... 입니다!

YerinCho

안녕하세요 럿고선생님 알트선생님이 피드백을 맛깔나게 해주셨군요
그래서 저는 매우 간단한 피드백 남겼어요.
우선 안되는 기능 구현하시고 피드백 반영 해주세요~

YerinCho · 2020-06-29T03:02:34Z

src/main/java/domain/payment/CardPayment.java

+		long beveragePaymentAmount = orderHistories.getOrderHistories()
+			.stream()
+			.filter(orderHistory -> orderHistory.isSameCategory(Category.BEVERAGE))
+			.map(OrderHistory::calculatePaymentAmount)
+			.mapToLong(n -> n)
+			.sum();


Suggested change

long beveragePaymentAmount = orderHistories.getOrderHistories()

.stream()

.filter(orderHistory -> orderHistory.isSameCategory(Category.BEVERAGE))

.map(OrderHistory::calculatePaymentAmount)

.mapToLong(n -> n)

.sum();

long beveragePaymentAmount = orderHistories.getOrderHistories()

.stream()

.filter(orderHistory -> orderHistory.isSameCategory(Category.BEVERAGE))

.mapToLong(OrderHistory::calculatePaymentAmount)

.sum();

YerinCho · 2020-06-29T03:11:55Z

src/main/java/domain/payment/CashPayment.java

+			.map(OrderHistory::calculatePaymentAmount)
+			.mapToLong(n -> n)


Suggested change

.map(OrderHistory::calculatePaymentAmount)

.mapToLong(n -> n)

.mapToLong(OrderHistory::calculatePaymentAmount)

YerinCho · 2020-06-29T03:15:44Z

src/main/java/domain/payment/CashPayment.java

+		return disCountStrategies.stream()
+			.filter(disCountStrategyValue -> disCountStrategyValue.getClass() == disCountStrategy)
+			.findFirst();
+	}


알트 의견 동의합니다~ 우선 보이는건
calculatePaymentAmountOfChicken, calculatePaymentAmountOfBeverage, findDisCountStrategy 정도 , ,?

YerinCho · 2020-06-29T03:19:34Z

src/main/java/domain/table/OrderHistories.java

+			.map(OrderHistory::getQuantity)
+			.mapToInt(n -> n)


Suggested change

.map(OrderHistory::getQuantity)

.mapToInt(n -> n)

.mapToInt(OrderHistory::getQuantity)

YerinCho · 2020-06-29T03:28:55Z

src/test/java/domain/table/OrderHistoriesTest.java

+		assertThatThrownBy(() -> orderHistories.add(new OrderHistory(chicken, 100)))
+			.isInstanceOf(IllegalArgumentException.class)
+			.hasMessage("한 메뉴당 99까지만 주문할 수 있습니다. menu = [치킨] 1 - 후라이드 : 16000원");
+	}


처음 주문할 때 99개를 주문하면 주문이 잘 안되네요 ㅜㅜ
테스트 케이스 추가하면서 수정해주시면 좋을 것 같아요~

조건을 잘못줬어요 ㅠㅠ 수정했습니다!

YerinCho · 2020-06-29T03:31:39Z

src/main/java/domain/table/OrderHistories.java

@@ -0,0 +1,53 @@
+package domain.table;


패키지를 옮겼지만 기존 domain 내에 있던 Table, TableRepo, Menu, MenuRepo가 아직 남아있는 것 같아요~

2019로 진행해서 머지하면서 들어간거 같아요! 삭제했습니다!

YerinCho · 2020-06-29T03:35:58Z

src/test/java/domain/table/OrderHistoriesTest.java

+		assertThatThrownBy(() -> orderHistories.add(new OrderHistory(chicken, 100)))
+			.isInstanceOf(IllegalArgumentException.class)
+			.hasMessage("한 메뉴당 99까지만 주문할 수 있습니다. menu = [치킨] 1 - 후라이드 : 16000원");
+	}


처음 99개 미만으로 주문하고, 그 다음 주문 수를 더한 합계가 99개를 넘는 경우 예외처리가 안 되어 있어요!
테스트 추가하고 프로덕션 코드에도 추가해주세요~

로직이 잘못되었네요 ㅠㅠ 수정햇습니다!

pobiconan and others added 26 commits April 10, 2020 13:14

initial commit

a3fcdc8

docs : 기능 요구 사항 및 제약 사항 작성

5571561

feat: 기능 도메인 생성

30681f7

feactor : gitignore 설정

ce9762e

feat: 문자열을 숫자로 바꾸는 컨버터 생성 및 메인화면 view와 기능 선택 구현

9763112

feat : 컨트롤러 분리

7f4adfc

feat : 메뉴 서비스, 테이블 서비스 구현

6fa3996

feat : 테이블 번호, 메뉴 번호, 메뉴 수량 입력 받는 기능 구현

7dc4e22

feat: 주문 내역 도메인 생성

6103407

feat : 테이블에 주문내역이 등록되는 기능 구현

2b1ff80

feat : 주문 내역 출력

daf6cf2

feat : 결제 방법 입력 받는 기능 구현

4b063eb

feat: 결제 방식 도메인 생성

8ed612e

feat : 카드 결제 구현

61e4df0

refactor : BigInteger를 BigDecimal로 변경

63eb9cd

refactor : 주문내역 일급컬렉션 생성 및 치킨 갯수에 맞춰 할인 구현

a88841b

test : tableRepository Test 작성

9d03a11

feat : 주문이 있는 테이블 표시 기능 구현

69f2f22

feat : 결제 후 결제 내역 삭제 기능 구현

e2ebd9a

refactor : 전반적인 컨벤션 수정

1c8d943

test : Category toString() 테스트 구현

e0adf3a

test : menu Test 구현

d06563a

feat: 주문 수량이 99개가 넘어가면 예외처리

4c8824d

refactor : 할인 전략 구현

4f382ed

refactor : 사용하지 않는 import 제거

fb12dde

merge branch 'tdd-rutgo' of https://github.com/ksy90101/java-chicken-…

6505c88

…2020 into tdd-rutgo-pr

KS-KIM reviewed Jun 27, 2020

View reviewed changes

YerinCho reviewed Jun 29, 2020

View reviewed changes

SeYun added 2 commits June 30, 2020 16:37

refactor : 레포, 서비스, 컨트롤러 싱글톤으로 관리

7241334

refacotr : repo에서 optional 반환으로 변경

7f8dec3

test: 테이블 숫자가 없는걸 찾으려고 할때 예외처리 테스트 추가

acf1349

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[럿고] 치킨집 미션 제출합니다. #5

[럿고] 치킨집 미션 제출합니다. #5

ksy90101 commented Jun 27, 2020 •

edited

Loading

KS-KIM left a comment

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020

KS-KIM Jun 27, 2020

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020

YerinCho left a comment

YerinCho Jun 29, 2020

YerinCho Jun 29, 2020

YerinCho Jun 29, 2020

YerinCho Jun 29, 2020

YerinCho Jun 29, 2020

ksy90101 Jun 30, 2020

YerinCho Jun 29, 2020

ksy90101 Jun 30, 2020

YerinCho Jun 29, 2020

ksy90101 Jun 30, 2020

		Payment payment = Payment.of(paymentNumber);
		BigDecimal account = payment.pay(table.getOrderHistories());

	private Optional<DisCountStrategy> findDisCountStrategy(Class<? extends DisCountStrategy> disCountStrategy) {
	private Optional<DisCountStrategy> findDiscountStrategy(Class<? extends DisCountStrategy> disCountStrategy) {


		class OrderHistoriesTest {

		private final MenuRepository menuRepository = new MenuRepository();

		.map(OrderHistory::calculatePaymentAmount)
		.mapToLong(n -> n)

	.map(OrderHistory::calculatePaymentAmount)
	.mapToLong(n -> n)
	.mapToLong(OrderHistory::calculatePaymentAmount)

	.map(OrderHistory::getQuantity)
	.mapToInt(n -> n)
	.mapToInt(OrderHistory::getQuantity)

[럿고] 치킨집 미션 제출합니다. #5

Are you sure you want to change the base?

[럿고] 치킨집 미션 제출합니다. #5

Conversation

ksy90101 commented Jun 27, 2020 • edited Loading

KS-KIM left a comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

ksy90101 Jun 30, 2020 • edited Loading

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YerinCho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksy90101 commented Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

ksy90101 Jun 30, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading

KS-KIM Jun 27, 2020 •

edited

Loading