Lauren Steenkamp test#8
Open
Lauren-Robyn wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request attempts to implement solutions for a Python programming assessment involving string parsing, algorithmic logic, and test-driven development. The PR includes implementations for five functions and a test file for the reactor status functionality.
Key Changes:
- Implementation of
get_departure_airport()to extract airport codes from ticket strings - Implementation of
check_baggage_allowance(),validate_flight_number(),is_leap_year(), and partial implementation ofreactor_status() - Creation of
test_reactor.pytest file (incomplete)
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| test_reactor.py | New test file created for reactor_status function, but contains multiple syntax errors and incomplete test implementation |
| system_check.py | Implements solutions for 5 programming challenges with critical bugs in logic, incorrect use of print vs return, and incomplete implementations |
| pycache/test_system_check.cpython-312.pyc | Compiled bytecode (no review needed) |
| pycache/test_reactor.cpython-312.pyc | Compiled bytecode (no review needed) |
| pycache/system_check.cpython-312.pyc | Compiled bytecode (no review needed) |
Critical Issues Found:
- Multiple syntax errors in test_reactor.py (missing function arguments, incorrect string comparison, missing parentheses)
- Functions using print() instead of return statements as specified in docstrings
- Incorrect leap year logic implementation
- Incomplete reactor_status function
- Logic errors in validate_flight_number (wrong operator usage, incorrect type checking)
- Debug/test code left in production file
Comments suppressed due to low confidence (2)
test_reactor.py:9
- Comparison of constants; use 'True' or 'False' instead.
if '__name__' == '__main__':
test_reactor.py:9
- Testing a constant will always give the same result.
if '__name__' == '__main__':
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+10
| ) | ||
| if '__name__' == '__main__': | ||
| unittest.main No newline at end of file |
There was a problem hiding this comment.
The assertEqual call is missing both arguments. It should compare an actual value with an expected value, e.g., self.assertEqual(reactor_status(100, 50), "Maintenance Mode").
Suggested change
| ) | |
| if '__name__' == '__main__': | |
| unittest.main | |
| reactor_status(100, 50), "Maintenance Mode" | |
| ) | |
| if __name__ == '__main__': | |
| unittest.main() |
Comment on lines
+9
to
+10
| if '__name__' == '__main__': | ||
| unittest.main No newline at end of file |
There was a problem hiding this comment.
The condition check is incorrect. '__name__' is a string literal, not a variable. It should be if __name__ == '__main__': (without quotes around __name__).
Suggested change
| if '__name__' == '__main__': | |
| unittest.main | |
| if __name__ == '__main__': | |
| unittest.main() |
| # TODO: Write your code here | ||
| pass | ||
| flight_number = ticket_string.split("-")[1] | ||
| print(flight_number) |
There was a problem hiding this comment.
This debug print statement should be removed from the production code.
Suggested change
| print(flight_number) |
Comment on lines
+75
to
+83
| for i in flight_number: | ||
| direction_val = 0 | ||
| if i is int: | ||
| direction_val += i | ||
|
|
||
| if direction_val / 2 == 0 : | ||
| print("Valid - Northbound") | ||
|
|
||
| elif direction_val / 2 != 0 : |
There was a problem hiding this comment.
The variable direction_val is being initialized inside the loop on each iteration. It should be initialized once before the loop starts: move this line before line 75.
Suggested change
| for i in flight_number: | |
| direction_val = 0 | |
| if i is int: | |
| direction_val += i | |
| if direction_val / 2 == 0 : | |
| print("Valid - Northbound") | |
| elif direction_val / 2 != 0 : | |
| direction_val = 0 | |
| for i in flight_number: | |
| if i.isdigit(): | |
| direction_val += int(i) | |
| if direction_val % 2 == 0 and direction_val != 0: | |
| print("Valid - Northbound") | |
| elif direction_val % 2 != 0: |
Comment on lines
+88
to
89
| validate_flight_number("FL-JO234-JNB-CPT-2023") | ||
|
|
There was a problem hiding this comment.
This function call should be removed. It appears to be debugging/test code left in the production file and will execute every time the module is imported.
Suggested change
| validate_flight_number("FL-JO234-JNB-CPT-2023") |
| pass | ||
|
|
||
|
|
||
| #TODO: Write your code here |
There was a problem hiding this comment.
Missing space after # in the comment. Should be # TODO: for consistency with code style.
Suggested change
| #TODO: Write your code here | |
| # TODO: Write your code here |
Comment on lines
+43
to
+56
| if ticket_string.split("-")[0] == "EC": | ||
| baggage_allowance = 20 | ||
| print("Economy - 20kg") | ||
| elif ticket_string.split("-")[0] == "BS": | ||
| baggage_allowance = 40 | ||
| print("Business - 40kg") | ||
| elif ticket_string.split("-")[0] == "FL": | ||
| baggage_allowance = 60 | ||
| print("First Class - 60kg") | ||
| else: | ||
| baggage_allowance = 0 | ||
| print("Standard - 0kg") | ||
| return baggage_allowance | ||
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") |
There was a problem hiding this comment.
Variable baggage_allowance is not used.
Suggested change
| if ticket_string.split("-")[0] == "EC": | |
| baggage_allowance = 20 | |
| print("Economy - 20kg") | |
| elif ticket_string.split("-")[0] == "BS": | |
| baggage_allowance = 40 | |
| print("Business - 40kg") | |
| elif ticket_string.split("-")[0] == "FL": | |
| baggage_allowance = 60 | |
| print("First Class - 60kg") | |
| else: | |
| baggage_allowance = 0 | |
| print("Standard - 0kg") | |
| return baggage_allowance | |
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") | |
| class_code = ticket_string.split("-")[0] | |
| if class_code == "EC": | |
| return "Economy - 20kg" | |
| elif class_code == "BS": | |
| return "Business - 40kg" | |
| elif class_code == "FL": | |
| return "First Class - 60kg" | |
| else: | |
| return "Standard - 0kg" | |
| print(check_baggage_allowance("FL-JO234-JNB-CPT-2023")) |
Comment on lines
+44
to
+56
| baggage_allowance = 20 | ||
| print("Economy - 20kg") | ||
| elif ticket_string.split("-")[0] == "BS": | ||
| baggage_allowance = 40 | ||
| print("Business - 40kg") | ||
| elif ticket_string.split("-")[0] == "FL": | ||
| baggage_allowance = 60 | ||
| print("First Class - 60kg") | ||
| else: | ||
| baggage_allowance = 0 | ||
| print("Standard - 0kg") | ||
| return baggage_allowance | ||
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") |
There was a problem hiding this comment.
Variable baggage_allowance is not used.
Suggested change
| baggage_allowance = 20 | |
| print("Economy - 20kg") | |
| elif ticket_string.split("-")[0] == "BS": | |
| baggage_allowance = 40 | |
| print("Business - 40kg") | |
| elif ticket_string.split("-")[0] == "FL": | |
| baggage_allowance = 60 | |
| print("First Class - 60kg") | |
| else: | |
| baggage_allowance = 0 | |
| print("Standard - 0kg") | |
| return baggage_allowance | |
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") | |
| return "Economy - 20kg" | |
| elif ticket_string.split("-")[0] == "BS": | |
| return "Business - 40kg" | |
| elif ticket_string.split("-")[0] == "FL": | |
| return "First Class - 60kg" | |
| else: | |
| return "Standard - 0kg" | |
| print(check_baggage_allowance("FL-JO234-JNB-CPT-2023")) |
Comment on lines
+44
to
+56
| baggage_allowance = 20 | ||
| print("Economy - 20kg") | ||
| elif ticket_string.split("-")[0] == "BS": | ||
| baggage_allowance = 40 | ||
| print("Business - 40kg") | ||
| elif ticket_string.split("-")[0] == "FL": | ||
| baggage_allowance = 60 | ||
| print("First Class - 60kg") | ||
| else: | ||
| baggage_allowance = 0 | ||
| print("Standard - 0kg") | ||
| return baggage_allowance | ||
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") |
There was a problem hiding this comment.
Variable baggage_allowance is not used.
Suggested change
| baggage_allowance = 20 | |
| print("Economy - 20kg") | |
| elif ticket_string.split("-")[0] == "BS": | |
| baggage_allowance = 40 | |
| print("Business - 40kg") | |
| elif ticket_string.split("-")[0] == "FL": | |
| baggage_allowance = 60 | |
| print("First Class - 60kg") | |
| else: | |
| baggage_allowance = 0 | |
| print("Standard - 0kg") | |
| return baggage_allowance | |
| check_baggage_allowance("FL-JO234-JNB-CPT-2023") | |
| return "Economy - 20kg" | |
| elif ticket_string.split("-")[0] == "BS": | |
| return "Business - 40kg" | |
| elif ticket_string.split("-")[0] == "FL": | |
| return "First Class - 60kg" | |
| else: | |
| return "Standard - 0kg" | |
| print(check_baggage_allowance("FL-JO234-JNB-CPT-2023")) |
Comment on lines
+2
to
+3
| from system_check import reactor_status | ||
|
|
There was a problem hiding this comment.
Import of 'reactor_status' is not used.
Suggested change
| from system_check import reactor_status |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.