-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[로또] 김성규 미션 제출합니다. #4
base: main
Are you sure you want to change the base?
Conversation
Walkthrough이 변경 사항은 외부 라이브러리 임포트 검사를 위한 새 함수 추가, 복권 시스템 관련 기능의 확장 및 개선, 그리고 테스트 설정 보완을 포함합니다. GitHub 워크플로우에 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant M as 메인 함수
participant L as Lotto 클래스
U->>M: 구매 금액 입력
M->>M: check_amount & prompt_purchase_amount 처리
M->>L: 티켓 생성 (generate_num 호출)
U->>M: 당첨 번호 입력
M->>M: prompt_winning_numbers 처리
U->>M: 보너스 번호 입력
M->>M: prompt_bonus_number 처리
M->>M: evaluate_tickets 호출하여 결과 계산
M->>U: print_results로 결과 출력
sequenceDiagram
participant W as 워크플로우
participant F as 파일 (src)
W->>F: 각 파일 읽기
W->>W: AST 파싱 후 import 문 검사
alt 외부 라이브러리 감지됨
W->>W: SystemExit 발생하여 중단
else
W->>W: 다음 파일로 계속 진행
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/check-no-external-libs.yml (1)
25-30
: 🛠️ Refactor suggestion불필요한 공백 제거 및 함수 개선
- 30번 줄의 불필요한 공백을 제거해야 합니다.
- 모듈 경로 검사 로직을 개선할 수 있습니다.
def is_internal_module(module_name): """ 내부 모듈(`src/` 폴더 내 Python 파일)인지 확인 """ module_path = os.path.join('src', module_name.replace('.', '/') + '.py') module_dir = os.path.join('src', module_name.replace('.', '/')) return os.path.exists(module_path) or os.path.isdir(module_dir) -
추가로 다음과 같은 개선사항을 제안드립니다:
def is_internal_module(module_name): """ 내부 모듈(`src/` 폴더 내 Python 파일)인지 확인 """ paths = [ os.path.join('src', module_name.replace('.', '/') + '.py'), os.path.join('src', module_name.replace('.', '/'), '__init__.py') ] return any(os.path.exists(path) for path in paths)🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 30-30: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (9)
src/lotto/lotto.py (4)
6-8
: [초기화 메서드에 대한 간단한 문서화 추가를 제안합니다]
__init__
메서드의 파라미터numbers: list[int]
는 Python 3.9 이상 버전에서 유효합니다. 이 요구사항을 사용 설명서 혹은 docstring에 명시해 주시면 좋을 것 같습니다.
10-16
: [예외 메시지 내용과 조건의 불일치를 수정해 주세요]
아래 조건에서는 6개가 “아닐” 때 예외를 발생시키지만, 메시지에는 6개가 “넘어가면”이라고 쓰여 있습니다. 실제 의도에 맞게 메시지를 수정해 주세요.예시 수정안:
-raise ValueError("로또 번호의 개수가 6개가 넘어가면 예외가 발생한다.") +raise ValueError("로또 번호는 정확히 6개여야 합니다.")🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-16: src/lotto/lotto.py#L16
Added line #L16 was not covered by tests
18-20
: [클래스 메서드 인자명 개선을 권장합니다]
메서드 정의 시clss
대신cls
라는 관용적인 이름을 사용해 주시면 가독성이 조금 더 좋아질 수 있습니다.
25-26
: [문자열 표현을 명확하게 할 수도 있습니다]
__str__
에 대한 문자열 형태를 좀 더 사용자 친화적으로 구성할 수 있습니다. 예:"Lotto numbers: [1, 2, 3, 4, 5, 6]"
.src/lotto/main.py (2)
36-44
: [보너스 번호 검증 로직 리뷰]
중복 및 범위 감지 로직이 적절하고 메시지도 사용자 친화적입니다. 추가로보너스 번호가 0 또는 음수일 때
등 다른 극단 사례도 테스트에 반영해 보시면 좋겠습니다.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 38-38: src/lotto/main.py#L38
Added line #L38 was not covered by tests
[warning] 40-40: src/lotto/main.py#L40
Added line #L40 was not covered by tests
[warning] 42-42: src/lotto/main.py#L42
Added line #L42 was not covered by tests🪛 GitHub Actions: Check PEP8 Style
[error] 36-36: Expected 2 blank lines, found 1
68-83
: [출력 형식 및 PEP8]
print_results
내에서 수익률 계산을 반올림하여 출력하는 로직이 유용합니다. 한편, PEP8 검사에서 줄 길이와 빈 줄 관련 지적이 다수 확인되었으므로, 코드 포맷팅을 수정해 주세요.🧰 Tools
🪛 GitHub Actions: Check PEP8 Style
[error] 68-68: Expected 2 blank lines, found 1
docs/README.md (1)
1-24
: [문서 업데이트가 명확하고 일관성 있습니다]
각 단계를 구체적으로 설명하고 있어 사용자 입장에서 기능을 파악하기 좋습니다. 5번 항목 ‘예외 처리’ 중에 다른 항목들과 맞춰 개조식 표현(“5. 예외 처리” 뒤에 한 칸 띄우기 등)을 좀 더 맞추면 형식적으로 통일감이 있을 것 같습니다..github/workflows/check-no-external-libs.yml (2)
23-23
: 표준 라이브러리 허용 목록 확장 검토현재 허용된 모듈 목록이 기본적인 작업을 위해 충분하지만, 다음과 같은 유용한 표준 라이브러리 모듈들도 고려해 보시는 것을 추천드립니다:
json
: JSON 데이터 처리collections
: 특수 컨테이너 타입typing
: 타입 힌트- allowed_modules = {'sys', 'os', 'math', 'random', 'datetime', 're', 'enum'} + allowed_modules = {'sys', 'os', 'math', 'random', 'datetime', 're', 'enum', 'json', 'collections', 'typing'}
31-41
: import 검사 로직 개선 제안현재 구현은 기본적인 기능은 잘 수행하고 있지만, 다음과 같은 개선사항들을 고려해보시기 바랍니다:
- 파일 읽기 오류 처리
- 모든 위반 사항을 수집하여 한 번에 보고
- 더 자세한 오류 메시지 제공
def check_imports(file_path): violations = [] try: with open(file_path, 'r', encoding='utf-8') as f: tree = ast.parse(f.read(), filename=file_path) for node in ast.walk(tree): if isinstance(node, ast.Import): for alias in node.names: if alias.name not in allowed_modules and not is_internal_module(alias.name): violations.append(f'직접 import: {alias.name} (라인: {node.lineno})') elif isinstance(node, ast.ImportFrom): if node.module and node.module not in allowed_modules and not is_internal_module(node.module): violations.append(f'from import: {node.module} (라인: {node.lineno})') if violations: raise SystemExit( f'❌ {file_path}에서 외부 라이브러리 사용이 감지되었습니다:\n' + '\n'.join(f' - {v}' for v in violations) ) except Exception as e: raise SystemExit(f'❌ {file_path} 파일 처리 중 오류 발생: {str(e)}')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/check-no-external-libs.yml
(1 hunks)docs/README.md
(1 hunks)pytest.ini
(0 hunks)src/lotto/lotto.py
(1 hunks)src/lotto/main.py
(1 hunks)
💤 Files with no reviewable changes (1)
- pytest.ini
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/check-no-external-libs.yml
[error] 30-30: trailing spaces
(trailing-spaces)
🪛 GitHub Check: codecov/patch
src/lotto/lotto.py
[warning] 16-16: src/lotto/lotto.py#L16
Added line #L16 was not covered by tests
src/lotto/main.py
[warning] 7-7: src/lotto/main.py#L7
Added line #L7 was not covered by tests
[warning] 9-9: src/lotto/main.py#L9
Added line #L9 was not covered by tests
[warning] 33-34: src/lotto/main.py#L33-L34
Added lines #L33 - L34 were not covered by tests
[warning] 38-38: src/lotto/main.py#L38
Added line #L38 was not covered by tests
[warning] 40-40: src/lotto/main.py#L40
Added line #L40 was not covered by tests
[warning] 42-42: src/lotto/main.py#L42
Added line #L42 was not covered by tests
[warning] 50-51: src/lotto/main.py#L50-L51
Added lines #L50 - L51 were not covered by tests
🪛 GitHub Actions: Check PEP8 Style
src/lotto/main.py
[error] 3-3: Expected 2 blank lines, found 1
[error] 12-12: Expected 2 blank lines, found 1
[error] 22-22: Expected 2 blank lines, found 1
[error] 27-27: Expected 2 blank lines, found 1
[error] 36-36: Expected 2 blank lines, found 1
[error] 45-45: Expected 2 blank lines, found 1
[error] 53-53: Expected 2 blank lines, found 1
[error] 68-68: Expected 2 blank lines, found 1
[error] 84-84: Expected 2 blank lines, found 1
[error] 92-92: Line too long (80 > 79 characters)
[error] 95-95: Expected 2 blank lines after class or function definition, found 1
🔇 Additional comments (8)
src/lotto/lotto.py (2)
22-23
: [정상 동작 확인]
get_numbers
메서드는 변환 로직 없이 내부 자료만 반환하므로, 현재 로직상 문제없이 동작해 보입니다.
29-47
: [추가 테스트를 권장합니다]
Rank.get_rank
는 정상적으로 순회 후 매칭되는 열거형을 반환해 주지만, 6개가 모두 일치하면서 보너스도 일치하는 특이 케이스(이론상 실제 1등보다 더 높은 케이스)를 다루지 않으니, 해당 상황에 대한 테스트도 한 번 고려해 보세요.src/lotto/main.py (6)
3-11
: [입력 검증 로직에 대한 테스트 보강을 권장합니다]
check_amount
함수에서 문자열이 숫자인지 검사하고, 1000원 단위인지 확인하는 로직은 적절합니다. 다만,int(input_amount)
가 정확히 0인 경우나 예상치 못한 입력 형태에 대해 커버리지가 부족할 수 있으니 추가 테스트를 고려해 주세요.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 7-7: src/lotto/main.py#L7
Added line #L7 was not covered by tests
[warning] 9-9: src/lotto/main.py#L9
Added line #L9 was not covered by tests🪛 GitHub Actions: Check PEP8 Style
[error] 3-3: Expected 2 blank lines, found 1
22-26
: [티켓 출력 기능]
print_lotto_tickets
는 단순 반복 출력이므로 특별한 문제가 없어 보입니다.🧰 Tools
🪛 GitHub Actions: Check PEP8 Style
[error] 22-22: Expected 2 blank lines, found 1
27-35
: [당첨 번호 입력 예외 상황 테스트 보강 제안]
당첨 번호 입력에서ValueError
가 발생할 수 있는 시나리오(예: 7개 이상 입력, 중복, 범위 벗어남 등)에 대한 테스트를 더 늘려서 커버리지를 높여 보시면 좋겠습니다.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 33-34: src/lotto/main.py#L33-L34
Added lines #L33 - L34 were not covered by tests🪛 GitHub Actions: Check PEP8 Style
[error] 27-27: Expected 2 blank lines, found 1
45-52
: [보너스 번호 재입력 흐름 검토]
prompt_bonus_number
함수는 예외 발생 시 계속 입력을 받도록while True
루프를 유지하므로,check_bonus_number
에서 예외 발생 시 로직 흐름이 적절히 이어집니다. 현재 구조는 좋아 보입니다.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-51: src/lotto/main.py#L50-L51
Added lines #L50 - L51 were not covered by tests🪛 GitHub Actions: Check PEP8 Style
[error] 45-45: Expected 2 blank lines, found 1
53-67
: [당첨 티켓 평가 로직 확인]
evaluate_tickets
함수는 교집합을 통한 매칭 계산과 보너스 확인이 깔끔히 구성되어 있습니다. 추가로, 전체 로직에 예외 상황(티켓이 비어있는 경우 등)을 테스트해 보면 안정성을 더 높일 수 있습니다.🧰 Tools
🪛 GitHub Actions: Check PEP8 Style
[error] 53-53: Expected 2 blank lines, found 1
85-93
: [메인 함수 로직이 간결합니다]
메인 흐름이 함수들을 순차적으로 호출하여 로또 게임을 실행하는 구조가 적절합니다.🧰 Tools
🪛 GitHub Actions: Check PEP8 Style
[error] 92-92: Line too long (80 > 79 characters)
def prompt_purchase_amount(): | ||
while True: | ||
try: | ||
print("구입금액을 입력해 주세요.") | ||
amount = input() | ||
return check_amount(amount) | ||
except ValueError as error: | ||
print(f"[ERROR] {error}") | ||
raise | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[입력이 유효하지 않을 때 반복 구조가 중단됩니다]
이 부분에서 ValueError
가 발생하면 raise
로 인해 while True
루프를 빠져나갑니다. 계속 입력을 받으려면 raise
대신 사용자에게 재입력을 유도하는 방식(continue
)을 고려해 보세요.
-except ValueError as error:
- print(f"[ERROR] {error}")
- raise
+except ValueError as error:
+ print(f"[ERROR] {error}")
+ continue
📝 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.
def prompt_purchase_amount(): | |
while True: | |
try: | |
print("구입금액을 입력해 주세요.") | |
amount = input() | |
return check_amount(amount) | |
except ValueError as error: | |
print(f"[ERROR] {error}") | |
raise | |
def prompt_purchase_amount(): | |
while True: | |
try: | |
print("구입금액을 입력해 주세요.") | |
amount = input() | |
return check_amount(amount) | |
except ValueError as error: | |
print(f"[ERROR] {error}") | |
continue |
🧰 Tools
🪛 GitHub Actions: Check PEP8 Style
[error] 12-12: Expected 2 blank lines, found 1
Summary by CodeRabbit