Keamogetswe's submission#2
Conversation
There was a problem hiding this comment.
Pull request overview
This submission implements solutions for a Python formative assessment covering flight ticket parsing, leap year logic, and a reactor safety monitor with test-driven development.
Key changes:
- Implemented four core functions in
system_check.py: departure airport extraction, baggage allowance checking, flight number validation, and leap year determination - Added
reactor_statusfunction with conditional logic for nuclear reactor safety monitoring - Created
test_reactor.pywith unit tests for the reactor status function (TDD requirement)
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| system_check.py | Implements all five required functions with string parsing, modulo operations, and conditional logic; includes debug print statements |
| test_reactor.py | Provides unit tests for reactor_status function covering critical, warning, and normal operation scenarios |
| .vscode/settings.json | Configures VS Code to use unittest as the testing framework |
| pycache/*.pyc | Generated bytecode files from Python execution |
Comments suppressed due to low confidence (5)
system_check.py:24
- Print statement may execute during import.
print(get_departure_airport("FL-JO234-JNB-CPT-2023"))
system_check.py:54
- Print statement may execute during import.
print(check_baggage_allowance("FL-JO234-JNB-CPT-2023"))
system_check.py:78
- Print statement may execute during import.
print(validate_flight_number("FL-A9-JNB-CPT-2023"))
system_check.py:109
- Print statement may execute during import.
print(is_leap_year(1900))
system_check.py:152
- Print statement may execute during import.
print(reactor_status(1500, 200))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| depature_airport_code = new_string[2] | ||
| return depature_airport_code |
There was a problem hiding this comment.
Typo in variable name: "depature" should be "departure".
| depature_airport_code = new_string[2] | |
| return depature_airport_code | |
| departure_airport_code = new_string[2] | |
| return departure_airport_code |
| new_string = ticket_string.split("-") | ||
| depature_airport_code = new_string[2] | ||
| return depature_airport_code | ||
| print(get_departure_airport("FL-JO234-JNB-CPT-2023")) |
There was a problem hiding this comment.
This debug print statement should be removed from production code. Print statements at the module level can clutter output and are typically only used during development/debugging.
| number_portion = int(flight_number.replace("JO", "").replace("A", "")) | ||
| if number_portion % 2 == 0: | ||
| return "Valid - Northbound" | ||
| if number_portion % 2 != 0: | ||
| return "Valid - Southbound" | ||
| # I don't know what's wrong with this part of the code, I lost my brain cells!!! | ||
| if number_portion.isdigit() == False: | ||
| return "Invalid Flight" |
There was a problem hiding this comment.
This logic has a critical flaw. The code attempts to convert to int first (line 70), which will raise a ValueError if the conversion fails. This means lines 76-77 (which check if it's not a digit) are unreachable code - if the string contains non-numeric characters, the function will crash at line 70 before reaching the validation check.
The correct approach is to:
- Extract the numeric portion as a string
- Check if it's a valid digit using
isdigit() - Only then convert to int
For example:
number_str = flight_number.replace("JO", "").replace("A", "")
if not number_str.isdigit():
return "Invalid Flight"
number_portion = int(number_str)| number_portion = int(flight_number.replace("JO", "").replace("A", "")) | |
| if number_portion % 2 == 0: | |
| return "Valid - Northbound" | |
| if number_portion % 2 != 0: | |
| return "Valid - Southbound" | |
| # I don't know what's wrong with this part of the code, I lost my brain cells!!! | |
| if number_portion.isdigit() == False: | |
| return "Invalid Flight" | |
| number_str = flight_number.replace("JO", "").replace("A", "") | |
| if not number_str.isdigit(): | |
| return "Invalid Flight" | |
| number_portion = int(number_str) | |
| if number_portion % 2 == 0: | |
| return "Valid - Northbound" | |
| else: | |
| return "Valid - Southbound" |
| # I don't know what's wrong with this part of the code, I lost my brain cells!!! | ||
| if number_portion.isdigit() == False: | ||
| return "Invalid Flight" | ||
| print(validate_flight_number("FL-A9-JNB-CPT-2023")) |
There was a problem hiding this comment.
This debug print statement should be removed from production code. Print statements at the module level can clutter output and are typically only used during development/debugging.
| return True | ||
| else: | ||
| return False | ||
| print(is_leap_year(1900)) |
There was a problem hiding this comment.
This debug print statement should be removed from production code. Print statements at the module level can clutter output and are typically only used during development/debugging.
| number_portion = int(flight_number.replace("JO", "").replace("A", "")) | ||
| if number_portion % 2 == 0: | ||
| return "Valid - Northbound" | ||
| if number_portion % 2 != 0: |
There was a problem hiding this comment.
Consider using elif instead of separate if statements for the mutually exclusive northbound/southbound conditions. Since a number can't be both even and odd, using elif improves code clarity.
For example:
if number_portion % 2 == 0:
return "Valid - Northbound"
else: # or elif number_portion % 2 != 0:
return "Valid - Southbound"| if number_portion % 2 != 0: | |
| elif number_portion % 2 != 0: |
| import unittest | ||
| from system_check import reactor_status | ||
|
|
||
| class TestReactor: |
There was a problem hiding this comment.
The TestReactor class must inherit from unittest.TestCase for the test methods to work properly. Without this inheritance, the test runner won't recognize this as a test class, and methods like self.assertEqual will fail since they are not defined.
Change:
class TestReactor:To:
class TestReactor(unittest.TestCase):| class TestReactor: | |
| class TestReactor(unittest.TestCase): |
| return "First Class - 60kg" | ||
| else: | ||
| return "Standard - 0kg" | ||
| print(check_baggage_allowance("FL-JO234-JNB-CPT-2023")) |
There was a problem hiding this comment.
This debug print statement should be removed from production code. Print statements at the module level can clutter output and are typically only used during development/debugging.
| return "Maintenance Mode" | ||
| else: | ||
| return "Normal Operation" | ||
| print(reactor_status(1500, 200)) |
There was a problem hiding this comment.
This debug print statement should be removed from production code. Print statements at the module level can clutter output and are typically only used during development/debugging.
| if baggage_allowance == 'BS': | ||
| return "Business - 40kg" | ||
| if baggage_allowance == 'FL': |
There was a problem hiding this comment.
Consider using elif instead of multiple if statements for mutually exclusive conditions. This improves readability and performance by avoiding unnecessary condition checks once a match is found.
For example:
if baggage_allowance == 'EC':
return "Economy - 20kg"
elif baggage_allowance == 'BS':
return "Business - 40kg"
elif baggage_allowance == 'FL':
return "First Class - 60kg"
else:
return "Standard - 0kg"| if baggage_allowance == 'BS': | |
| return "Business - 40kg" | |
| if baggage_allowance == 'FL': | |
| elif baggage_allowance == 'BS': | |
| return "Business - 40kg" | |
| elif baggage_allowance == 'FL': |
No description provided.