Skip to content

Conversation

@seeyoujeong
Copy link

@seeyoujeong seeyoujeong commented Jul 15, 2024

추가 요구사항 구현 목록

  • 로또 번호는 오름차순으로 정렬하여 보여준다.
  • 당첨 통계를 출력한 뒤에는 재시작/종료 여부를 입력받는다.
  • 재시작할 경우 구입 금액 입력부터 게임을 다시 시작하고, 종료하는 경우 그대로 프로그램을 종료시킨다.
  • 사용자가 잘못된 값을 입력한 경우 throw문을 사용해 예외를 발생시키고, 에러 메시지를 출력 후 그 부분부터 입력을 다시 받는다.

미션을 수행하면서 어려웠던 점

  • 예외 처리 위치 정하기
    기존에는 inputManagerretyrScan 메서드의 콜백에서 에러를 처리하는 형식으로 했었는데, 이렇게 구현하다보니 예외 처리에 한계를 느꼈습니다. 그래서 retryOnFailureAsync라는 메서드를 만들어서 외부에서 처리를 해줄 수 있게 수정했는데, 이게 맞는지 애매하네요...
  • MVC 패턴
    shouldRestartGame, handleErrorAndPrint 메서드를 어디에 넣는게 좋을지 고민을 많이 했네요.

리뷰 받고 싶은 부분 & 궁금한 점

  • 레이어
    각 레이어에 속한 함수나 클래스의 위치가 괜찮은지 의견 공유 부탁드립니다.
  • 가독성
    클래스 내부의 메서드들 혹은 파일의 메서드들의 나열된 순서가 코드를 읽을때 괜찮은지, 이름은 이해가 되는지 궁금합니다!
  • 분리
    최대한 분리를 했지만 더 분리할 코드가 있을까요?

어떤 지점에서 재미를 많이 느끼는지, 희열을 많이 느끼는지, 몰입을 많이 하는지

  • 재미와 희열
    사실 코딩하면서 재미와 희열까지 느껴본 기억은 없는듯하고, 구현을 끝내거나 코드 정리가 깔끔하게 되면 뿌듯함을 느낍니다.
  • 몰입
    하기 싫지 않은 상태에서 특정 시간 내에 끝낼 수 있을 정도의 목표를 정하고 일을 시작하면 몰입이 되는 것 같아요.

Summary by CodeRabbit

  • New Features

    • Implemented the Lotto Mission feature, including purchasing tickets, drawing winning numbers, and displaying results.
    • Added extensive validation for lotto numbers and error handling with user-friendly messages.
  • Tests

    • Introduced comprehensive test cases for all classes involved in the Lotto Mission, ensuring robustness and reliability.
  • Scripts

    • Updated test scripts in package.json for better test management.
  • Documentation

    • Updated the README to reflect the new Lotto Mission functionality.

@seeyoujeong seeyoujeong requested a review from JunilHwang July 15, 2024 15:16
@seeyoujeong seeyoujeong self-assigned this Jul 15, 2024
@coderabbitai
Copy link

coderabbitai bot commented Jul 15, 2024

Walkthrough

The project has shifted from a Car Racing Mission to a Lotto Mission, encompassing updates in various files to support this change. This includes modifications to README, test scripts, and the implementation of new classes for managing lotto functionalities such as ticket generation, result calculation, and user interaction. The new functionalities are backed by extensive test suites to ensure accuracy and reliability.

Changes

File/Path Change Summary
README.md Updated the document to reflect the change from Car Racing Mission to Lotto Mission.
package.json Modified test script and added a test:watch script.
src/__tests__/Lotto.test.js Introduced tests for the Lotto class.
src/__tests__/LottoMachine.test.js Introduced tests for the LottoMachine class.
src/__tests__/LottoMatcher.test.js Introduced tests for the LottoMatcher class.
src/__tests__/LottoNumber.test.js Introduced tests for the LottoNumber class.
src/__tests__/LottoReturnCalculator.test.js Introduced tests for the LottoReturnCalculator class.
src/__tests__/WinningLotto.test.js Introduced tests for the WinningLotto class.
src/constants/errorMessage.js Introduced user-friendly error messages for lotto input validation.
src/constants/index.js Exported all constants related to the lotto game.
src/constants/lotto.js Defined various properties related to the lotto game.
src/controller/calculateWinningResults.js Introduced a function to calculate lotto winning results.
src/controller/createWinningLotto.js Introduced functionality to create a winning lotto object.
src/controller/handleErrorAndPrint.js Introduced a function to handle and print errors.
src/controller/index.js Exported various lotto management functions.
src/controller/purchaseLottos.js Introduced functionality for purchasing lotto tickets.
src/controller/shouldRestartGame.js Introduced functionality to confirm if the game should be restarted.
src/main.js Significantly altered to handle purchasing and calculating lotto results.
src/model/Lotto.js Introduced the Lotto class with input validation and number retrieval.
src/model/LottoMachine.js Introduced the LottoMachine class for creating and managing lotto instances.
src/model/LottoMatcher.js Introduced the LottoMatcher class for calculating winning counts.
src/model/LottoNumber.js Introduced the LottoNumber class for handling and validating lotto numbers.
src/model/LottoReturnCalculator.js Introduced the LottoReturnCalculator class to calculate the rate of return for the lotto game.
src/model/WinningLotto.js Introduced the WinningLotto class extending Lotto with bonus number validation.
src/model/index.js Exported several modules related to the Lotto system.
src/service/index.js Exported inputManager and outputManager from corresponding files.
src/service/inputManager.js Introduced the InputManager class for handling user input.
src/service/outputManager.js Introduced the OutputManager class for handling output operations.
src/utils/arrayUtils.js Introduced utility functions for generating ranges and shuffling arrays.
src/utils/formatKoreanCurrency.js Introduced a function to format numbers into Korean currency.
src/utils/formatNumber.js Introduced a function to format numbers to a specified number of digits.
src/utils/getUserFriendlyErrorMessage.js Introduced a function to map error codes to user-friendly messages.
src/utils/index.js Exported various utility functions.
src/utils/isDuplicated.js Introduced a function to check for duplicate elements in an array.
src/utils/readLineAsync.js Introduced a function to read user input asynchronously using the readline module.
src/utils/retryOnFailureAsync.js Introduced a function to retry an async function on failure.
src/utils/validate.js Introduced validation utility functions for types, integers, and arrays.
src/view/index.js Exported showPurchasedLottos and showWinningResults functions.
src/view/showPurchasedLottos.js Introduced a function to display purchased lotto tickets.
src/view/showWinningResults.js Introduced a function to display lotto winning results.

Sequence Diagrams

Lotto Purchase Flow

sequenceDiagram
    participant User
    participant InputManager
    participant LottoMachine
    participant OutputManager
    
    User->>InputManager: Enter purchase amount
    InputManager->>LottoMachine: Create lotto machine with amount
    LottoMachine->>LottoMachine: Generate lottos
    LottoMachine->>OutputManager: Display purchased lottos
    OutputManager->>User: Show purchased lottos
Loading

Lotto Winning Calculation Flow

sequenceDiagram
    participant User
    participant InputManager
    participant LottoMachine
    participant LottoMatcher
    participant LottoReturnCalculator
    participant OutputManager

    User->>InputManager: Enter winning numbers
    InputManager->>LottoMatcher: Calculate matching lottos
    LottoMatcher->>LottoReturnCalculator: Calculate return rate
    LottoReturnCalculator->>OutputManager: Display winning results
    OutputManager->>User: Show winning results
Loading

Poem

In the lotto's dance, we play,
Numbers shuffle, come what may.
Tickets bought with hopeful cheer,
Winning dreams are drawing near.
🎟️✨
As the numbers roll and spin,
Who will lose and who will win?
This new mission's just begun,
May your luck shine like the sun! 🌞


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (2)
src/utils/getUserFriendlyErrorMessage.js (1)

1-8: Consider using optional chaining.

The getUserFriendlyErrorMessage function is correctly implemented. However, you can simplify the code by using optional chaining.

-  const errorCode = match && match[1];
+  const errorCode = match?.[1];
Tools
Biome

[error] 3-3: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/service/outputManager.js (1)

12-19: Improve the typeof check.

The typeof check for processFn can be improved by using a more explicit check for function type.

-        typeof processFn === "function" ? processFn(value) : value;
+        typeof processFn === 'function' ? processFn(value) : value;
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c602cd4 and 19ab847.

Files selected for processing (40)
  • README.md (1 hunks)
  • package.json (1 hunks)
  • src/tests/Lotto.test.js (1 hunks)
  • src/tests/LottoMachine.test.js (1 hunks)
  • src/tests/LottoMatcher.test.js (1 hunks)
  • src/tests/LottoNumber.test.js (1 hunks)
  • src/tests/LottoReturnCalculator.test.js (1 hunks)
  • src/tests/WinningLotto.test.js (1 hunks)
  • src/constants/errorMessage.js (1 hunks)
  • src/constants/index.js (1 hunks)
  • src/constants/lotto.js (1 hunks)
  • src/controller/calculateWinningResults.js (1 hunks)
  • src/controller/createWinningLotto.js (1 hunks)
  • src/controller/handleErrorAndPrint.js (1 hunks)
  • src/controller/index.js (1 hunks)
  • src/controller/purchaseLottos.js (1 hunks)
  • src/controller/shouldRestartGame.js (1 hunks)
  • src/main.js (1 hunks)
  • src/model/Lotto.js (1 hunks)
  • src/model/LottoMachine.js (1 hunks)
  • src/model/LottoMatcher.js (1 hunks)
  • src/model/LottoNumber.js (1 hunks)
  • src/model/LottoReturnCalculator.js (1 hunks)
  • src/model/WinningLotto.js (1 hunks)
  • src/model/index.js (1 hunks)
  • src/service/index.js (1 hunks)
  • src/service/inputManager.js (1 hunks)
  • src/service/outputManager.js (1 hunks)
  • src/utils/arrayUtils.js (1 hunks)
  • src/utils/formatKoreanCurrency.js (1 hunks)
  • src/utils/formatNumber.js (1 hunks)
  • src/utils/getUserFriendlyErrorMessage.js (1 hunks)
  • src/utils/index.js (1 hunks)
  • src/utils/isDuplicated.js (1 hunks)
  • src/utils/readLineAsync.js (1 hunks)
  • src/utils/retryOnFailureAsync.js (1 hunks)
  • src/utils/validate.js (1 hunks)
  • src/view/index.js (1 hunks)
  • src/view/showPurchasedLottos.js (1 hunks)
  • src/view/showWinningResults.js (1 hunks)
Files skipped from review due to trivial changes (11)
  • README.md
  • src/constants/index.js
  • src/constants/lotto.js
  • src/controller/handleErrorAndPrint.js
  • src/controller/index.js
  • src/model/index.js
  • src/service/index.js
  • src/utils/formatKoreanCurrency.js
  • src/utils/index.js
  • src/view/index.js
  • src/view/showPurchasedLottos.js
Additional context used
Biome
src/utils/getUserFriendlyErrorMessage.js

[error] 3-3: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/utils/validate.js

[error] 9-9: Invalid typeof comparison value: this expression is not a string literal

not a string literal

(lint/suspicious/useValidTypeof)

src/model/LottoMatcher.js

[error] 63-63: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (84)
src/utils/isDuplicated.js (1)

1-7: LGTM!

The isDuplicated function is correctly implemented and efficiently checks for duplicates in an array.

src/utils/formatNumber.js (1)

1-5: LGTM!

The formatNumber function is correctly implemented and efficiently formats the number to the specified number of digits.

src/utils/retryOnFailureAsync.js (1)

1-11: LGTM!

The retryOnFailureAsync function is correctly implemented and efficiently retries the asynchronous function on failure.

src/constants/errorMessage.js (4)

2-3: LGTM!

The error message is clear and provides specific instructions.


4-4: LGTM!

The error message is clear and provides specific instructions.


5-6: LGTM!

The error message is clear and provides specific instructions.


7-7: LGTM!

The error message is clear and provides specific instructions.

package.json (2)

6-6: LGTM!

The addition of the "type" field is appropriate for enabling ECMAScript modules.


10-11: LGTM!

The addition of the test scripts is appropriate and follows best practices.

src/controller/shouldRestartGame.js (2)

3-7: LGTM!

The function is straightforward and correctly handles user input.


11-17: LGTM!

The function is straightforward and correctly handles user input.

src/utils/arrayUtils.js (2)

1-15: LGTM!

The function is straightforward and correctly generates the array.


17-22: LGTM!

The function is straightforward and correctly shuffles the array.

src/service/inputManager.js (3)

1-2: Import statement looks good.

The import statement correctly imports readLineAsync from the utils.


3-15: Class and methods are well-implemented.

The InputManager class and its methods are implemented correctly. The scan method handles asynchronous input and processes it with a callback function if provided.


17-19: Exporting an instance is appropriate.

Exporting an instance of InputManager is a good approach for singleton usage.

src/__tests__/LottoReturnCalculator.test.js (2)

1-2: Import statements look good.

The import statements correctly import the necessary modules for testing and the LottoReturnCalculator class.


4-19: Test suite is well-implemented.

The test suite correctly verifies the calculation of the rate of return. The test data and assertions are appropriate.

src/controller/calculateWinningResults.js (3)

1-2: Import statements look good.

The import statements correctly import the necessary constants and classes.


4-12: Function implementation looks good.

The calculateWinningResults function correctly calculates the winning counts and rate of return. The logic is clear and appropriate.


14-14: Export statement looks good.

The export statement correctly exports the calculateWinningResults function.

src/utils/readLineAsync.js (3)

1-2: Import statement looks good.

The import statement correctly imports the readline module.


3-19: Function implementation looks good.

The readLineAsync function correctly handles asynchronous reading of input. The error handling for non-string queries is appropriate.


21-21: Export statement looks good.

The export statement correctly exports the readLineAsync function.

src/utils/validate.js (1)

1-5: LGTM!

The function throwErrorWithCondition is implemented correctly.

src/service/outputManager.js (3)

4-6: LGTM!

The constructor is implemented correctly.


8-10: LGTM!

The print method is implemented correctly.


21-23: LGTM!

The linebreak method is implemented correctly.

src/main.js (3)

1-7: LGTM!

The import statements are correct.


9-22: Add a validity check for the purchaseLottos response.

Ensure that the response from purchaseLottos is valid before proceeding.

-    const { count, lottos } = await purchaseLottos();
+    const { count, lottos } = await purchaseLottos();
+    if (!count || !lottos) {
+      throw new Error('Invalid response from purchaseLottos');
+    }

23-23: LGTM!

The main function invocation is correct.

src/controller/purchaseLottos.js (3)

1-4: LGTM!

The import statements are correct.


6-18: Add input validation for the scanned value.

Ensure that the input value is a valid number before converting it.

-        const trimedInputValue = inputValue.trim();
+        const trimmedInputValue = inputValue.trim();
+        if (isNaN(trimmedInputValue) || trimmedInputValue <= 0) {
+          throw new Error('Invalid input value');
+        }
-        return Number(trimedInputValue);
+        return Number(trimmedInputValue);

23-23: LGTM!

The export statement is correct.

src/model/WinningLotto.js (3)

7-13: Constructor looks good!

The constructor correctly initializes the WinningLotto class and validates the bonus number.


15-17: Getter method looks good!

The getter method correctly returns the value of the #bonusNumber property.


19-24: Validation method looks good!

The static method correctly validates that the bonus number is not duplicated in the winning numbers.

src/__tests__/WinningLotto.test.js (2)

5-12: Test case for bonus number validation looks good!

The test case correctly checks that an error is thrown if the bonus number is duplicated in the winning numbers.


14-21: Test case for bonus number retrieval looks good!

The test case correctly checks that the bonus number can be retrieved.

src/view/showWinningResults.js (1)

5-21: Function looks good! Verify the necessity of the reverse method.

The function correctly displays the winning results and rate of return using the outputManager. Verify if the reverse method is necessary on LOTTO.RANKING_INFO.

Verification successful

The reverse method is necessary.

The RANKING_INFO array is defined in descending order by default. The reverse method is used to iterate over the array in ascending order, which is necessary for displaying the results correctly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the `reverse` method is necessary on `LOTTO.RANKING_INFO`.

# Test: Check the sorting order of `LOTTO.RANKING_INFO`. Expect: Already sorted in the required order.
rg --type js 'RANKING_INFO' -A 5

Length of output: 1419


Script:

#!/bin/bash
# Extract the complete definition of `RANKING_INFO` from `src/constants/lotto.js`.
rg 'RANKING_INFO' -A 10 src/constants/lotto.js

Length of output: 256


Script:

#!/bin/bash
# Extract more lines from `src/constants/lotto.js` to get the complete definition of `RANKING_INFO`.
rg 'RANKING_INFO' -A 50 src/constants/lotto.js

Length of output: 516

src/model/LottoNumber.js (3)

7-11: Constructor looks good!

The constructor correctly initializes the LottoNumber class and validates the number.


13-15: Getter method looks good!

The getter method correctly returns the value of the #number property.


17-31: Validation method looks good!

The static method correctly validates that the number is an integer and within the valid range.

src/model/LottoReturnCalculator.js (3)

1-12: LGTM!

The constructor and class properties are correctly implemented.


14-16: LGTM!

The rateOfReturn method is correctly implemented.


18-32: LGTM!

The static methods #calcRateOfReturn and #getPrizeMoney are correctly implemented.

src/model/Lotto.js (3)

1-16: LGTM!

The constructor and class properties are correctly implemented.


18-20: LGTM!

The numbers method is correctly implemented.


22-37: LGTM!

The static method #validateNumbers is correctly implemented.

src/__tests__/Lotto.test.js (4)

1-19: LGTM!

The test cases for invalid constructor arguments are correctly implemented.


21-25: LGTM!

The test case for constructor argument length exceeding 6 is correctly implemented.


27-31: LGTM!

The test case for constructor argument length less than 6 is correctly implemented.


33-37: LGTM!

The test case for retrieving lotto numbers is correctly implemented.

src/__tests__/LottoNumber.test.js (4)

1-20: LGTM!

The test cases for invalid constructor arguments are correctly implemented.


22-26: LGTM!

The test case for constructor argument less than 1 is correctly implemented.


28-32: LGTM!

The test case for constructor argument greater than 45 is correctly implemented.


34-40: LGTM!

The test case for setting lotto number is correctly implemented.

src/__tests__/LottoMachine.test.js (4)

5-8: LGTM!

The test case correctly checks for an error when a non-integer value is passed to the LottoMachine constructor.


11-14: LGTM!

The test case correctly checks for an error when a value less than 1000 is passed to the LottoMachine constructor.


17-29: LGTM!

The test case correctly checks if the correct number of lottos are issued for various input prices using test.each.


31-37: LGTM!

The test case correctly checks if a lotto contains 6 numbers.

src/model/LottoMachine.js (5)

13-26: LGTM!

The constructor and properties correctly initialize the LottoMachine and provide access to the number of lottos and the lottos themselves.


28-37: LGTM!

The #validatePrice method correctly validates that the price is an integer and is at least 1000.


40-42: LGTM!

The #countLotto method correctly calculates the number of lottos that can be purchased with the given price.


44-53: LGTM!

The #createLottos and #createLotto methods correctly create an array of lottos and generate a single lotto with sorted numbers.


55-59: LGTM!

The #getLottoNumbers method correctly generates a shuffled array of lotto numbers.

src/model/LottoMatcher.js (5)

6-15: LGTM!

The constructor and properties correctly initialize the LottoMatcher and provide access to the winning counts.


17-35: LGTM!

The #calculateWinningCounts method correctly calculates the winning counts for the given lottos and winningLotto.


37-39: LGTM!

The #getMatchingCount method correctly calculates the number of matching numbers between a lotto and the winningLotto.


42-44: LGTM!

The #updateWinningCounts method correctly updates the winning counts based on the lotto match results.


46-53: LGTM!

The #updateCounts method correctly updates the counts based on the matching count and bonus match.

src/controller/createWinningLotto.js (5)

6-15: LGTM!

The createWinningLotto function correctly uses retryOnFailureAsync to create a winning lotto with retry logic.


19-26: LGTM!

The retryCreateWinningNumbers function correctly uses retryOnFailureAsync to create winning numbers with retry logic.


28-41: LGTM!

The createWinningNumbers function correctly handles user input for winning numbers and creates a Lotto instance.


44-51: LGTM!

The retryCreateBonusNumber function correctly uses retryOnFailureAsync to create a bonus number with retry logic.


53-65: LGTM!

The createBonusNumber function correctly handles user input for the bonus number and creates a LottoNumber instance.

src/__tests__/LottoMatcher.test.js (8)

1-8: Imports look good!

The import statements are correctly added and necessary for the test cases.


10-28: Test case looks good!

The test case correctly verifies the properties and types of the winningCounts object.


30-37: Test case looks good!

The test case correctly verifies the first-place winning count.


39-46: Test case looks good!

The test case correctly verifies the second-place winning count.


48-55: Test case looks good!

The test case correctly verifies the third-place winning count.


57-64: Test case looks good!

The test case correctly verifies the fourth-place winning count.


66-73: Test case looks good!

The test case correctly verifies the fifth-place winning count.


76-85: Function looks good!

The createLottoMachineMock function correctly creates a mock LottoMachine with the provided lotto numbers.

Comment on lines +56 to +63
static #getRanking(matchingCount, isBonusMatch) {
const currentInfo = LOTTO.RANKING_INFO.find(
(info) =>
info.matchingCount === matchingCount &&
Boolean(info.isBonusMatch) === isBonusMatch
);

return currentInfo && currentInfo.ranking;
Copy link

Choose a reason for hiding this comment

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

Use optional chaining for better readability.

The #getRanking method can be improved by using optional chaining.

-    return currentInfo && currentInfo.ranking;
+    return currentInfo?.ranking;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static #getRanking(matchingCount, isBonusMatch) {
const currentInfo = LOTTO.RANKING_INFO.find(
(info) =>
info.matchingCount === matchingCount &&
Boolean(info.isBonusMatch) === isBonusMatch
);
return currentInfo && currentInfo.ranking;
static #getRanking(matchingCount, isBonusMatch) {
const currentInfo = LOTTO.RANKING_INFO.find(
(info) =>
info.matchingCount === matchingCount &&
Boolean(info.isBonusMatch) === isBonusMatch
);
return currentInfo?.ranking;
Tools
Biome

[error] 63-63: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Comment on lines +7 to +10
export const validate = {
type(value, typeValue, errorMessage) {
throwErrorWithCondition(typeof value !== typeValue, errorMessage);
},
Copy link

Choose a reason for hiding this comment

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

Fix the typeof comparison.

The typeof comparison should use string literals.

-    throwErrorWithCondition(typeof value !== typeValue, errorMessage);
+    throwErrorWithCondition(typeof value !== typeValue.toLowerCase(), errorMessage);

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 9-9: Invalid typeof comparison value: this expression is not a string literal

not a string literal

(lint/suspicious/useValidTypeof)

@seeyoujeong seeyoujeong changed the base branch from main to seeyoujeong July 15, 2024 15:23
@JunilHwang
Copy link
Contributor

AI가 해주는 리뷰입니다

1. 코드 분석

이 Pull Request는 로또 게임의 기본 구조를 구현하고 있습니다. 주요 특징은 다음과 같습니다:

  • MVC 패턴을 적용하여 View, Controller, Model(Domain)을 분리했습니다.
  • 유효성 검사를 위한 Validator 클래스들을 구현했습니다.
  • 단위 테스트를 통해 주요 기능들을 검증하고 있습니다.
  • ESLint와 Prettier를 도입하여 코드 스타일을 일관되게 유지하고 있습니다.

2. 강점

  1. 모듈화와 관심사 분리

    • View, Controller, Domain 로직을 잘 분리하여 각 부분의 책임을 명확히 했습니다.
    • Validator를 별도로 분리하여 재사용성을 높였습니다.
  2. 테스트 주도 개발(TDD) 적용

    • Lotto와 WinLotto 클래스에 대한 단위 테스트를 작성하여 코드의 신뢰성을 높였습니다.
  3. 확장성 고려

    • createValidator 함수를 통해 다양한 유효성 검사 로직을 쉽게 추가할 수 있도록 했습니다.
  4. 코드 품질 관리

    • ESLint와 Prettier 설정을 통해 일관된 코드 스타일을 유지하고 있습니다.

3. 개선점

  1. 에러 처리 개선

    • 현재는 console.log로 에러를 출력하고 있습니다. 사용자 경험 향상을 위해 더 구체적인 에러 메시지와 처리 방식이 필요합니다.
  2. 테스트 커버리지 확대

    • LottoController와 View 클래스에 대한 단위 테스트가 부족합니다. 이 부분에 대한 테스트를 추가하면 좋겠습니다.
  3. 상수 관리

    • LottoRule.js에 있는 상수들을 별도의 설정 파일로 분리하면 관리가 더 용이할 것 같습니다.
  4. 비동기 처리 개선

    • readLineAsync 함수에서 reject 처리 후에도 코드가 계속 실행됩니다. 이 부분을 개선하면 좋겠습니다.

4. 구체적 제안

  1. 에러 처리 개선

    try {
      // 기존 코드
    } catch (error) {
      console.error('오류가 발생했습니다:', error.message);
      // 사용자에게 친화적인 메시지 출력
      this.view.printError(error.message);
    }
  2. 테스트 커버리지 확대

    • LottoController에 대한 테스트 추가:
    describe('LottoController', () => {
      test('getLottos should return correct number of lottos', async () => {
        // 테스트 코드 작성
      });
      // 다른 메서드에 대한 테스트 추가
    });
  3. 상수 관리

    • config.js 파일 생성:
    export const LOTTO_CONFIG = {
      PRICE: 1000,
      LIMIT_PRICE: 10_000_000,
      MIN_NUMBER: 1,
      MAX_NUMBER: 45,
      // ... 기타 상수들
    };
  4. 비동기 처리 개선

    export default function readLineAsync(query) {
      return new Promise((resolve, reject) => {
        if (arguments.length !== 1 || typeof query !== 'string') {
          return reject(new Error('Invalid arguments'));
        }
        // ... 기존 코드
      });
    }

5. 학습 포인트

  1. 단일 책임 원칙(SRP): 각 클래스와 함수가 하나의 책임만을 가지도록 설계하는 방법을 학습할 수 있습니다.
  2. 의존성 주입: LottoController 생성자에서 의존성을 주입받는 방식을 통해 느슨한 결합을 구현하는 방법을 배울 수 있습니다.
  3. 테스트 주도 개발(TDD): Lotto와 WinLotto 클래스에 대한 단위 테스트를 통해 TDD 방식의 개발을 경험할 수 있습니다.
  4. 비동기 프로그래밍: readline 모듈을 Promise로 래핑하여 사용하는 방식을 통해 비동기 프로그래밍 패턴을 학습할 수 있습니다.

6. 향후 요구사항 변경 대비

  1. 로또 규칙 변경 대응:

    • 현재 LottoRule.js에 정의된 규칙들이 변경될 경우를 대비해, 이 부분을 더욱 모듈화하고 설정 파일로 분리하는 것이 좋겠습니다.
    • 예를 들어, 로또 번호의 범위나 당첨 금액이 변경되는 경우를 고려해 봅시다:
    // config.js
    export const LOTTO_CONFIG = {
      NUMBER_RANGE: { MIN: 1, MAX: 45 },
      PRIZE_MONEY: {
        FIRST: 2_000_000_000,
        SECOND: 30_000_000,
        // ... 기타 등수별 상금
      }
    };

    이렇게 설정을 분리하면, 향후 규칙 변경 시 config.js 파일만 수정하면 되므로 유지보수가 용이해집니다.

  2. 새로운 로또 유형 추가:

    • 현재의 Lotto 클래스 구조를 더욱 추상화하여, 다양한 유형의 로또 게임을 쉽게 추가할 수 있도록 개선할 수 있습니다.
    • 예를 들어, 기본 로또와 평생품(예: 매주 자동 구매되는 로또) 등 새로운 유형의 로또가 추가될 경우를 고려해 봅시다:
    class BaseLotto {
      // 공통 로직
    }
    
    class StandardLotto extends BaseLotto {
      // 현재 구현된 로직
    }
    
    class LifetimeLotto extends BaseLotto {
      // 평생품 로또 특화 로직
    }
  3. 다국어 지원:

    • 현재 하드코딩된 메시지들을 별도의 언어 파일로 분리하여, 향후 다국어 지원이 필요할 때 쉽게 대응할 수 있도록 합니다.
    // messages.js
    export const MESSAGES = {
      ko: {
        PURCHASE_AMOUNT_PROMPT: '구매 금액을 입력해 주세요.',
       // ... 기타 메시지
      },
      en: {
        PURCHASE_AMOUNT_PROMPT: 'Please enter the purchase amount.',
        // ... 기타 메시지
      }
    };
  4. 성능 최적화:

    • 현재는 소규모 로또 게임에 적합한 구조입니다. 하지만 대량의 로또 번호를 처리해야 하는 경우, 성능 최적화가 필요할 수 있습니다.
    • 예를 들어, 대량의 로또 번호 생성 및 당첨 확인 로직을 최적화하는 방안을 고려해볼 수 있습니다:
    class LottoOptimizer {
      static generateBulkLottos(count) {
        // 대량의 로또 번호를 효율적으로 생성하는 로직
      }
    
      static checkWinningBulk(lottos, winningNumbers) {
        // 대량의 로또 번호에 대해 효율적으로 당첨 여부를 확인하는 로직
      }
    }

이러한 방식으로 코드를 구조화하면, 향후 요구사항 변경에 더욱 유연하게 대응할 수 있을 것입니다.

@JunilHwang
Copy link
Contributor

AI가 해주는 궁금한 점에 대한 답변입니다.

1. 레이어

전반적으로 레이어 구분이 잘 되어 있습니다. 하지만 몇 가지 개선할 점이 있습니다:

  1. Domain 레이어:

    • Lotto.js, WinLotto.js, LottoRule.js는 적절하게 위치해 있습니다.
    • LottoController.js는 Domain 레이어보다는 별도의 Controller 레이어로 분리하는 것이 좋겠습니다.
  2. View 레이어:

    • Input.jsOutput.js는 잘 위치해 있습니다.
    • View.js는 Input과 Output을 조합한 상위 레벨의 뷰로 보이므로 적절합니다.
  3. Util 레이어:

    • createValidator.js, formatCurrency.js, generateRandomNumber.js, readline.js는 잘 위치해 있습니다.
  4. Validator 레이어:

    • InputValidator.js, OutputValidator.js, LottoValidator.js는 잘 분리되어 있습니다.
    • 하지만 LottoValidator.js는 domain 폴더 아래에 있는데, 다른 Validator들과 함께 위치시키는 것이 일관성 있어 보입니다.

2. 가독성

전반적으로 메서드 이름과 순서는 이해하기 쉽고 논리적입니다. 몇 가지 제안:

  1. Lotto.js 및 WinLotto.js:

    • 메서드 순서: constructor -> static methods -> getter/setter -> instance methods
    • 현재 순서가 대체로 좋습니다. accord 메서드를 getRank 앞으로 옮기면 더 자연스러울 것 같습니다.
  2. LottoController.js:

    • 메서드 순서: constructor -> run -> getLottos -> getWinLotto -> getRestart
    • 현재 순서가 논리적이고 이해하기 쉽습니다.
  3. View.js:

    • 입력 관련 메서드들을 그룹화하고, 출력 관련 메서드들을 그룹화하면 더 읽기 쉬울 것 같습니다.
  4. Validator 파일들:

    • 각 validator 객체 내의 속성들을 알파벳 순서로 정렬하면 찾기 쉬울 것 같습니다.

3. 분리

코드가 이미 잘 분리되어 있지만, 다음과 같은 추가 분리를 고려해 볼 수 있습니다:

  1. LottoRule.js:

    • 상수와 함수를 별도의 파일로 분리할 수 있습니다. 예: LottoConstants.jsLottoUtils.js
  2. LottoController.js:

    • getLottos, getWinLotto, getRestart 메서드를 별도의 서비스 클래스로 분리할 수 있습니다. 예: LottoService.js
  3. View.js:

    • 입력 관련 메서드와 출력 관련 메서드를 별도의 클래스로 분리할 수 있습니다. 예: InputView.jsOutputView.js
  4. Validator:

    • 각 Validator 내의 검증 로직을 더 작은 단위의 함수로 분리할 수 있습니다. 이렇게 하면 재사용성과 테스트 용이성이 향상됩니다.
  5. Error Handling:

    • 에러 처리 로직을 별도의 ErrorHandler 클래스로 분리하여 중앙화된 에러 처리를 구현할 수 있습니다.

이러한 추가 분리는 코드의 책임을 더욱 명확히 하고, 테스트와 유지보수를 용이하게 만들 수 있습니다. 하지만 과도한 분리는 복잡성을 증가시킬 수 있으므로, 프로젝트의 규모와 요구사항을 고려하여 적절한 수준의 분리를 결정해야 합니다.

Copy link

@suyeon1218 suyeon1218 left a comment

Choose a reason for hiding this comment

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

안녕하세요 찬욱님...!! 찬욱님의 코드에선 controller를 유심히 봤습니다...! controller를 파일별로 분리하는 시도도 신기했는데 이건 이것 나름대로 잘 읽히는 것 같아요..! 도메인을 여러개로 나눠서 각각의 역할을 확실하게 분리하려고 한게 눈에 띕니다 ㅎㅎ! 열심히 고민하신 흔적인 것 같아요. AI리뷰는...찬욱님이 보시기엔 쓸만하신 것 같나요...?! (아직 못봄...)

예외처리 위치 정하기

  • 어떤 한계가 있었나요...?! 이렇게 쓰면 모든 input을 한번씩 다 래핑하면서 retry가 붙는 메서드 이름을 한번씩 다 만들어줘야 한다는 부분은 단점이 될 수도 있을 것 같아요. 에러처리의 한계를 극복하면서 외부에서 일일이 래핑하지 않아도 되는 방법은 없을까요...!

MVC 패턴

  • 찬욱님은 input과 관련한 건 모두 controller에 넣었는데...저는 view에 넣었습니다...! React에서 input을 받는다고 하면 Input컴포넌트를 통해 받는 걸 생각했는데 찬욱님 생각은 어떠신가요?

가독성

  • LottoMatcher를 읽을때 최초의 큰 함수에서 다른 함수로 계속 이동해가며 로직을 처리한다는 느낌을 받았어요. 함수가 하나의 역할만 하도록 분리하는 측면에선 매우 잘 분리하신 것 같은데, 이렇다 보니 처음 코드에서 반환받는 값은 결국 가장 아래있는 함수를 통해 받는 거니까... 어떤 값이 들어오게 될지 유추가 잘 안되는 단점도 있는 것 같아요. 함수를 역할별로 분리하면서 반환값을 유추할 수 있게 하는 게 너무 어려운 것 같아요...

레이어

각 도메인이 가진 역할 자체는 잘 분리했다는 생각이 듭니다! 정말 꼼꼼히 분리하셔서 이렇게도 분리할 수 있겠구나 감탄하면서 본 것 같아요!!

Comment on lines +3 to +6
const showPurchasedLottos = (count, lottos) => {
outputManager.print(`${count}개 구매했습니다.`);
outputManager.printAll(lottos, (lottos) => lottos.numbers);
};

Choose a reason for hiding this comment

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

뷰에서 도메인을 몰라야 하는 게 MVC의 원칙이라고 알고 있어요...! 그런데 여기선 lottos의 numbers를 참조하게 되면 도메인이 어떻게 이뤄져 있는지를 View가 알고 있다고 가정하게 되는 것 아닐까요...!

Copy link
Author

Choose a reason for hiding this comment

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

아 맞네요.. 중요한 부분 알려주셔서 감사합니다!

Comment on lines +1 to +5
export const range = (start, end) => {
if (end === undefined) {
end = start;
start = 0;
}

Choose a reason for hiding this comment

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

음수인 경우는 어떻게 처리하게 되나요?

Copy link
Author

Choose a reason for hiding this comment

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

열린 결말...?!

Comment on lines +7 to +12
const length = end - start;
const result = Array(length);

for (let i = 0; i < length; i++) {
result[i] = start + i;
}

Choose a reason for hiding this comment

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

이런 방식으로도 사용할 수 있을 것 같아요!

const result = [];

while (start < end) {
  result.push(start);
  start += 1;
}

아니면 이런 방식...?

const length = end - start;

return [...new Array(length)].map((el, index) => el = start + index);

Copy link
Author

@seeyoujeong seeyoujeong Jul 17, 2024

Choose a reason for hiding this comment

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

ㅎㅎ 사실 저 코드의 비밀은 ai에게 가장 빠른 속도를 가진 range 함수를 만들어달라고 했을 때 알려준 코드랍니다.
그래서 실제로 맞는지 테스트해보니까 저렇게 미리 배열을 만들어놓고 해당 인덱스에 입력하는게 속도가 빠르더라구요!

Comment on lines +2 to +3
ERR_001:
"입력하신 값은 로또 번호로 사용할 수 없습니다. 1부터 45 사이의 숫자를 입력해 주세요.",

Choose a reason for hiding this comment

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

에러를 번호로 구분하게 되면 에러를 명시적으로 작성하는 것과 달리 어떤 장점이 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

이렇게하면 개발자용 에러 메세지랑 사용자용 에러 메세지를 에러 코드를 통해 관리할 수 있어요!

Comment on lines +7 to +17
{
ranking: 1,
matchingCount: 6,
prizeMoney: 2_000_000_000,
},
{
ranking: 2,
matchingCount: 5,
isBonusMatch: true,
prizeMoney: 30_000_000,
},

Choose a reason for hiding this comment

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

isBonusMatch를 사용하지 않는다면 false를 명시하는 게 일관성을 해치지 않지 않을까요?!

Copy link
Author

Choose a reason for hiding this comment

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

저도 처음에는 그렇게 작성했는데 ai들이 없어도 된다고 해서...

Choose a reason for hiding this comment

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

controller를 여러개로 분리해놓으셨군요...! 이런 방법도 있을 수 있겠네요 🤔

Copy link
Author

Choose a reason for hiding this comment

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

사실 좋은 방법인지는 모르겠지만 지금거에서 좀더 수정해서 하나의 작업단위로 분리를 해볼까 합니다!

Comment on lines +34 to +35
return LottoMatcher.#updateWinningCounts(lottoMatchResults, initialCounts);
}

Choose a reason for hiding this comment

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

initialCounts를 calculateWinningCounts에선 사용하지 않고 updateWinningCounts로 넘겨준다면 updateWinningCounts에서 선언해도 되지 않을까요!

Copy link
Author

Choose a reason for hiding this comment

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

그래도 되긴하지만..! update하는 함수니까 값의 초기값을 들고 있는건 뭔가 안맞지 않을까 생각이 듭니다!

Comment on lines +37 to +40
static #getMatchingCount(lottoNumber, winningLottoNumber) {
return lottoNumber.filter((number) => winningLottoNumber.includes(number))
.length;
}

Choose a reason for hiding this comment

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

오홍... 이렇게 선언형으로도 일치하는 번호의 개수를 카운트해줄 수도 있군요

Comment on lines +49 to +51
if (ranking) {
counts[ranking] += 1;
}

Choose a reason for hiding this comment

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

getRanking에서 어떤 값을 반환하는지 궁금해지게 하는 제어문인것 같아요...!

Copy link
Author

Choose a reason for hiding this comment

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

좀 덜 명시적이긴 하죠..ㅎㅎ

constructor(price) {
LottoMachine.#validatePrice(price);

const count = LottoMachine.#countLotto(price);

Choose a reason for hiding this comment

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

어떤 count인지 처음읽을 때 헷갈려서 함수를 보고 나서 이해할 수 있었던 것 같아요...! lottoCount 혹은 purchasingCount 등으로 구매개수라는 사실을 명확히 나타내는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

로또 기계가 뭘하는지 보고 한번에 알긴 어렵겠군요..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants