[편의점] 정구홍 코드리뷰입니다. #1
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…그 부분부터 입력을 다시 받는다.
ghkgus
reviewed
Nov 15, 2024
ghkgus
left a comment
ghkgus
left a comment
There was a problem hiding this comment.
전체적으로 코드가 깔끔하고 이해하기 쉽게 작성된 것 같습니다. 배울 점이 많았던 코드였고, 4주 동안 수고 많으셨습니다!
|
|
||
| String[] parts = item.substring(START_INDEX, item.length() - END_INDEX_OFFSET).trim().split(ITEM_DELIMITER); | ||
| validateProductAndQuantityFormat(parts); | ||
|
|
There was a problem hiding this comment.
메서드가 조금 긴거 같아요 함수parts함수와 productName 메서드를 분리해, 검증해 반환하는 로직을 작성하는건 어떨까요?
|
|
||
|
|
||
| private static boolean isRemaining(int quantity) { | ||
| return quantity > MIN_QUANTITY; |
| outputView.printProducts(productDTOs); | ||
|
|
||
| Orders orders = retryUntilValid(() -> createOrders(productDTOs)); | ||
| Receipt receipt = processOrders(orders); |
There was a problem hiding this comment.
함수형 포인트를 사용하시니 코드가 정말 깔끔하네요.. 감탄하고 갑니다,,
| for (OrderDTO orderDTO : orders.getOrderDTOs()) { | ||
| OrderProcessor orderProcessor = new OrderProcessor(products, promotions, orderDTO); | ||
|
|
||
| if (orderProcessor.promotionIsNotAvailable()) { |
|
|
||
| private int getPromotionGetQuantity() { | ||
| return availableQuantity / (buy + get) * (get); | ||
| } |
There was a problem hiding this comment.
목적에 맞게 잘 나누 신거 같네요,, 저도 분리할껄 그랬나봐요ㅜㅜ 배우고 갑니다...
| } | ||
| } | ||
|
|
||
| public boolean promotionIsNotAvailable() { |
SeongUk52
reviewed
Nov 15, 2024
SeongUk52
left a comment
SeongUk52
left a comment
There was a problem hiding this comment.
DTO를 활용하여 깔끔하게 잘 구현하셨네요
마지막 미션 고생 많으셨습니다
| InputConverter inputConverter = new InputConverter(); | ||
|
|
||
| return new StoreController(inputView, outputView, inputConverter, products, promotions | ||
| ); |
Comment on lines
+25
to
+35
| public StoreController(InputView inputView, | ||
| OutputView outputView, | ||
| InputConverter inputConverter, | ||
| Products products, | ||
| Promotions promotions) { | ||
| this.inputView = inputView; | ||
| this.outputView = outputView; | ||
| this.inputConverter = inputConverter; | ||
| this.products = products; | ||
| this.promotions = promotions; | ||
| } |
Owner
Author
There was a problem hiding this comment.
다른 객체였다면 빌더 패턴 같은 것을 고려해보았을 것 같은데 컨트롤러다 보니 의존성 주입 시에 보여지는 부분을 신경쓰지 않은 것 같습니다.
Comment on lines
+5
to
+12
| public record PromotionDTO( | ||
| String name, | ||
| int buy, | ||
| int get, | ||
| LocalDate startDate, | ||
| LocalDate endDate | ||
| ) { | ||
| } |
Comment on lines
+29
to
+51
| public List<String> generateReceipt() { | ||
| List<String> receipt = new ArrayList<>(); | ||
| int totalAmount = 0; | ||
| int totalQuantity = 0; | ||
| int promotionDiscount = 0; | ||
| int membershipDiscount = 0; | ||
|
|
||
| addReceiptHeader(receipt); | ||
|
|
||
| int[] totals = addProductsToReceipt(receipt, totalAmount, totalQuantity); | ||
| totalAmount = totals[TOTAL_AMOUNT_INDEX]; | ||
| totalQuantity = totals[TOTAL_QUANTITY_INDEX]; | ||
|
|
||
| promotionDiscount = addPromotionsToReceipt(receipt, promotionDiscount); | ||
|
|
||
| if (applyMembershipDiscount) { | ||
| membershipDiscount = calculateMembershipDiscount(totalAmount, promotionDiscount); | ||
| } | ||
|
|
||
| addReceiptFooter(receipt, totalAmount, totalQuantity, promotionDiscount, membershipDiscount); | ||
|
|
||
| return receipt; | ||
| } |
yeong0jae
reviewed
Nov 17, 2024
| productDTO.quantity(), | ||
| productDTO.promotion()) | ||
| ); | ||
| } |
| if (input == null || input.trim().isEmpty()) { | ||
| throw new IllegalArgumentException(ErrorCode.GENERAL_INVALID_INPUT.getMessage()); | ||
| } | ||
| } |
| } | ||
|
|
||
| protected abstract T parseLine(String line); | ||
| } |
There was a problem hiding this comment.
새로운 데이터 형식이 추가되더라도 parseLine만 구현하면 된다는 구조가 너무 좋네요.
배워갑니다!!
|
|
||
| return String.format(PRODUCT_FORMAT_WITH_PROMOTION, name, price, quantity, promotion); | ||
| } | ||
| } |
There was a problem hiding this comment.
영수증 출력 로직 정말 깔끔하네요..!
모델에 의존하지 않고 데이터만 받아서 출력하는 것도 좋네요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.