lylet-AC / blockbuster

Repository containing code for the pygame game blockbuster.
GNU General Public License v3.0
1 stars 0 forks source link

Code Review of the blockbuster.py Module. #6

Open shafferz opened 5 years ago

shafferz commented 5 years ago

Author: Tyler Lyle

Reviewer: Zachary Shaffer

Date of the review: 14 December, 2018

Inspection rate: ~2.5 lines / minute (Total inspection time: ~1:03:00). Defect rate: 0.0/hr Defect density: 0.0

Checklist

Additional Comments

The code inspected was in the file blockbuster.py. The code's format was well done, and followed appropriate "Best Practices" for Python in an Object-Oriented format.

I had difficulty working through the functions on a line-by-line bases at close inspection due to a noticeable lack of commenting. Using my own experience with pygame, I was able to follow the general structure and format of each function. That being said, comments for clarification, especially in the functions load_data and new would help the Readability of the code greatly.

As I have not found any test cases, nor do I see a simple or direct way of writing the test cases for each of these functions (lack of return variables, no debugging statements to console, etc.), I am unsure of the testability of this project.

That aside, the code is rock-solid and performs its tasks excellently. The flow of the program is, after some reading and comprehension of the pygame API, fairly elegant and sensible. Very little (if any) of the code seems to be unstable or containing extraneous or defunct parts.

As the code functions perfectly in tandem with the other aspects of this project, and serves its purpose well, blockbuster.py can be considered usable by the metrics and scope of the overall project.

EDIT: Changed the checklist to reflect the commenting issue as one of Coding Best Practices, as opposed to Code Readability, given that the overall structure of the code is sensible.

lylet-AC commented 5 years ago

Accidentally did not push the correct version of this game to master, test cases have been added. Sorry! I also chose to remove all of the debug to console print lines, but I can add them back in.

shafferz commented 5 years ago

Excellent. I don't think they're necessary, but I do think you should have either them or, as stated in the original document, more comments. Otherwise, excellent work!