Cebolwethu Mzanywa#5
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements solutions for a coding assessment with five programming challenges covering data parsing, algorithmic logic, and test-driven development. However, the implementation is incomplete with several functions containing syntax errors and missing logic.
Key Changes:
- Added test suite
test_reactor.pyfor the reactor status monitoring function - Implemented
reactor_status()function with conditional logic for temperature and radiation monitoring - Implemented
is_leap_year()function with modulo logic for leap year calculation - Implemented
check_baggage_allowance()function for flight class baggage rules - Attempted partial implementations of
get_departure_airport()andvalidate_flight_number()functions (incomplete)
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| test_reactor.py | New unit test file for reactor_status function with 5 test cases covering error conditions, critical states, warnings, maintenance mode, and normal operation |
| system_check.py | Implementations for 5 coding challenges: 2 complete (is_leap_year, check_baggage_allowance), 1 mostly complete with minor bug (reactor_status), and 2 incomplete with syntax errors (get_departure_airport, validate_flight_number) |
| pycache/system_check.cpython-310.pyc | Compiled Python bytecode file that should not be committed to version control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import unittest | ||
| from system_check import reactor_status | ||
|
|
||
| class testReactor(unittest.TestCase): |
There was a problem hiding this comment.
Class name should follow PascalCase convention. The class name testReactor should be TestReactor to follow Python naming conventions for classes.
| class testReactor(unittest.TestCase): | |
| class TestReactor(unittest.TestCase): |
| #my_string = ticket_string[-10:-12] | ||
| my_list = [] | ||
| for i in ticket_string: | ||
| if |
There was a problem hiding this comment.
Incomplete if statement with syntax error. The condition after if is missing, which will cause a SyntaxError when the code is executed.
| if | |
| pass |
| #my_string = ticket_string[-10:-12] | ||
| my_list = [] | ||
| for i in ticket_string: | ||
| if | ||
| return |
There was a problem hiding this comment.
Incomplete return statement. The return statement is missing a value, which will return None instead of the expected departure airport code.
| #my_string = ticket_string[-10:-12] | |
| my_list = [] | |
| for i in ticket_string: | |
| if | |
| return | |
| # The departure airport code is between the second and third hyphens. | |
| parts = ticket_string.split('-') | |
| if len(parts) >= 4: | |
| return parts[2] | |
| return None |
| elif temp > 2000 or radiation > 500: | ||
| return f"CRITICAL" | ||
| elif (1000 <= temp <= 2000) and radiation > 100: | ||
| return f" WARNING" |
There was a problem hiding this comment.
Extra leading space in the return string. The return value " WARNING" has an extra space at the beginning, which should be "WARNING" to be consistent with other return values and to match the test expectations.
| return f" WARNING" | |
| return f"WARNING" |
| #re.search(r"", ticket_string) | ||
|
|
There was a problem hiding this comment.
Incomplete function implementation. The validate_flight_number function has no return statement and will return None instead of the expected validation result. The function needs to implement the logic to extract the flight number, check if the numeric part is even or odd, and return the appropriate result.
| return f"Economy - 20kg" | ||
| elif ticket_string[0:2] == "BS": | ||
| return f"Business - 40kg" | ||
| elif ticket_string[0:2] == "FL": | ||
| return f"First Class - 60kg" | ||
| else: | ||
| return f"Standard - 0kg" |
There was a problem hiding this comment.
[nitpick] Unnecessary f-string usage. The return statements in this function use f-strings without any variable interpolation. Consider using regular strings instead: return "Economy - 20kg" instead of return f"Economy - 20kg".
| return f"Economy - 20kg" | |
| elif ticket_string[0:2] == "BS": | |
| return f"Business - 40kg" | |
| elif ticket_string[0:2] == "FL": | |
| return f"First Class - 60kg" | |
| else: | |
| return f"Standard - 0kg" | |
| return "Economy - 20kg" | |
| elif ticket_string[0:2] == "BS": | |
| return "Business - 40kg" | |
| elif ticket_string[0:2] == "FL": | |
| return "First Class - 60kg" | |
| else: | |
| return "Standard - 0kg" |
| return f"Sensor Error" | ||
| elif temp > 2000 or radiation > 500: | ||
| return f"CRITICAL" | ||
| elif (1000 <= temp <= 2000) and radiation > 100: | ||
| return f" WARNING" | ||
| elif temp < 500: | ||
| return f"Maintenance Mode" | ||
| else: | ||
| return f"Normal Operation" |
There was a problem hiding this comment.
[nitpick] Unnecessary f-string usage. The return statements in this function use f-strings without any variable interpolation. Consider using regular strings instead for better readability and performance.
| return f"Sensor Error" | |
| elif temp > 2000 or radiation > 500: | |
| return f"CRITICAL" | |
| elif (1000 <= temp <= 2000) and radiation > 100: | |
| return f" WARNING" | |
| elif temp < 500: | |
| return f"Maintenance Mode" | |
| else: | |
| return f"Normal Operation" | |
| return "Sensor Error" | |
| elif temp > 2000 or radiation > 500: | |
| return "CRITICAL" | |
| elif (1000 <= temp <= 2000) and radiation > 100: | |
| return " WARNING" | |
| elif temp < 500: | |
| return "Maintenance Mode" | |
| else: | |
| return "Normal Operation" |
No description provided.