Skip to content

Comments

7주차 미션 구현 완료(박호건)#4

Open
moongua404 wants to merge 4 commits intoallrounder-backend:mainfrom
moongua404:moongua404
Open

7주차 미션 구현 완료(박호건)#4
moongua404 wants to merge 4 commits intoallrounder-backend:mainfrom
moongua404:moongua404

Conversation

@moongua404
Copy link
Contributor

No description provided.

@moongua404 moongua404 requested a review from Copilot May 27, 2025 12:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR completes the Week 7 mission by restructuring the lotto application into layers, introducing validation utilities, and expanding test coverage.

  • Introduce Validator utility for centralized input checks.
  • Extract core flow into LottoService with ports/adapters for I/O and randomness.
  • Add comprehensive tests covering error handling and result validation.

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/test/java/lotto/LottoTest.java Added import for new Lotto domain class.
src/test/java/lotto/ApplicationTest.java Introduced extensive tests for input errors, prize matching, and rate.
src/main/java/lotto/utils/Validator.java New validation methods for size, duplication, range, and divisibility.
src/main/java/lotto/application/service/LottoService.java Core service orchestrating input, generation, validation, and output.
src/main/java/lotto/application/port/outport/LottoPort.java Defined interface for output methods.
src/main/java/lotto/application/port/inport/GetRandomUseCase.java Interface for randomness provider.
src/main/java/lotto/application/port/inport/GetLottoPropertyUseCase.java Interface for reading user input.
src/main/java/lotto/application/domain/exceptions/ProgramTerminationException.java Custom exception for controlled termination.
src/main/java/lotto/application/domain/enums/MessageConstants.java Centralized message templates.
src/main/java/lotto/application/domain/enums/LottoPrize.java Defines prize tiers and matching logic.
src/main/java/lotto/application/domain/dto/WinningDataDto.java DTO for prize counts (currently unused).
src/main/java/lotto/application/domain/Lotto.java Domain entity with validation and match logic.
src/main/java/lotto/adapter/in/RandomLibrary.java Adapter for randomness using external utility.
src/main/java/lotto/adapter/in/InputTerminal.java Adapter for console input.
src/main/java/lotto/Lotto.java Removed old placeholder.
src/main/java/lotto/Application.java Bootstraps LottoService with adapters.
README.md Expanded assignment spec and usage instructions.
Comments suppressed due to low confidence (3)

src/main/java/lotto/utils/Validator.java:35

  • [nitpick] The phrasing is unclear. Consider updating the message to: "[ERROR] Amount must be divisible by 1,000."
throw new IllegalStateException("[ERROR] Invalid divided thousand.");

src/main/java/lotto/application/domain/Lotto.java:29

  • The parameter name numbers is misleading for a single number. Consider renaming it to number or value.
public boolean isDuplicate(int numbers) {

src/main/java/lotto/Application.java:10

  • [nitpick] Leftover TODO comment. The main implementation is in LottoService, so this can be removed for clarity.
// TODO: 프로그램 구현


public static <T> void hasNoDuplication(List<T> itemList) {
if (new HashSet<>(itemList).size() != itemList.size()) {
throw new IllegalStateException("[ERROR] Duplicated item exist.");
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

[nitpick] The error message has a grammar issue. Consider revising it to: "[ERROR] Duplicate items exist."

Suggested change
throw new IllegalStateException("[ERROR] Duplicated item exist.");
throw new IllegalStateException("[ERROR] Duplicate items exist.");

Copilot uses AI. Check for mistakes.
}
}

// TODO: 추가 기능 구현
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

Leftover TODO comment. Remove or implement the pending functionality to avoid dead code markers.

Suggested change
// TODO: 추가 기능 구현
public List<Integer> getSortedNumbers() {
return numbers.stream()
.sorted()
.toList();
}

Copilot uses AI. Check for mistakes.
Comment on lines +122 to +129
long sum = Stream.of(LottoPrize.FIFTH_PRICE, LottoPrize.FOURTH_PRICE, LottoPrize.THIRD_PRICE,
LottoPrize.SECOND_PRICE, LottoPrize.FIRST_PRICE)
.peek(prize ->
lottoPort.sendMessage(MessageConstants.RESULT_LINE,
prize.getCondition(),
prize.getPrice(),
result.getOrDefault(prize, 0L)))
.reduce(0L, (acc, prize) -> acc + (prize.getPrice() * result.getOrDefault(prize, 0L)), Long::sum);
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

[nitpick] Using peek for side effects (sending messages) within a stream that’s also reduced may hurt readability. Consider separating the messaging loop from the sum calculation.

Suggested change
long sum = Stream.of(LottoPrize.FIFTH_PRICE, LottoPrize.FOURTH_PRICE, LottoPrize.THIRD_PRICE,
LottoPrize.SECOND_PRICE, LottoPrize.FIRST_PRICE)
.peek(prize ->
lottoPort.sendMessage(MessageConstants.RESULT_LINE,
prize.getCondition(),
prize.getPrice(),
result.getOrDefault(prize, 0L)))
.reduce(0L, (acc, prize) -> acc + (prize.getPrice() * result.getOrDefault(prize, 0L)), Long::sum);
// Send messages for each LottoPrize
Stream.of(LottoPrize.FIFTH_PRICE, LottoPrize.FOURTH_PRICE, LottoPrize.THIRD_PRICE,
LottoPrize.SECOND_PRICE, LottoPrize.FIRST_PRICE)
.forEach(prize ->
lottoPort.sendMessage(MessageConstants.RESULT_LINE,
prize.getCondition(),
prize.getPrice(),
result.getOrDefault(prize, 0L))
);
// Calculate the total sum
long sum = Stream.of(LottoPrize.FIFTH_PRICE, LottoPrize.FOURTH_PRICE, LottoPrize.THIRD_PRICE,
LottoPrize.SECOND_PRICE, LottoPrize.FIRST_PRICE)
.mapToLong(prize -> prize.getPrice() * result.getOrDefault(prize, 0L))
.sum();

Copilot uses AI. Check for mistakes.
return price;
}

public static List<LottoPrize> getPrizeTypes() {
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

[nitpick] This method isn't used in LottoService, which hardcodes the prize sequence. Consider using this list to drive iterations and reduce duplication.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +6
package lotto.application.domain.dto;

import lotto.application.domain.enums.LottoPrize;

public record WinningDataDto(LottoPrize lottoPrize, int count) {
} No newline at end of file
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

[nitpick] This DTO isn't referenced anywhere. Either integrate it in LottoService or remove it to avoid dead code.

Suggested change
package lotto.application.domain.dto;
import lotto.application.domain.enums.LottoPrize;
public record WinningDataDto(LottoPrize lottoPrize, int count) {
}
// File removed as WinningDataDto is not referenced anywhere in the codebase.

Copilot uses AI. Check for mistakes.
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.

1 participant