Skip to content

Conversation

esundsmo
Copy link

No description provided.

@srli
Copy link

srli commented Apr 3, 2016

Review status: 0 of 6 files reviewed at latest revision, 10 unresolved discussions.


a discussion (no related file):
It's awesome how thoroughly everything is commented, much love :Db

I've made a couple of comments below at places where things could be improved, please take a look! Please remember to fill in the readme for future projects--it's really helpful for people who are unfamiliar with the code and is standard practice. I noticed that there weren't many commits to your git repo, version control is awesome, so take advantage of Git for your final project where there might be tons of people working on the same file at once.

Your code is very clean and easy to read, and the game runs smoothly. Good job!


brick_boop.py, line 43 [r3] (raw file):
Really loving all the in-line comments~


brick_boop.py, line 89 [r3] (raw file):
Why do you define Brick and Wall classes when they are identical to the Rectangle class?


brick_boop.py, line 182 [r3] (raw file):
A little nit-picky thing. Since you've already set self.width and self.height, you should use them in the initialization of self.ball and self.paddle


brick_boop.py, line 189 [r3] (raw file):
You seem to be creating a new rectangle with every call to update. You should make the Ball and Paddle objects instantiate the pygame.Rect instead of inside the model.update(). That way, you just have to change the .x and .y of the Rect instead of recreating it every loop.


brick_boop.py, line 196 [r3] (raw file):
Since the ball can only collide with one wall at a time, these should be if/elif statements


brick_boop.py, line 223 [r3] (raw file):
Note how all of these if statements are mostly identical, save for the segment of the paddle you're checking, and if self.ball.velo_x will be negative or positive. This could be simplified using something like a dictionary, or a for loop.


brick_boop.py, line 305 [r3] (raw file):
These should technically be if/elif statements


brick_boop.py, line 314 [r3] (raw file):
You don't need to float both the numerator and denominator to avoid getting rounding errors


brick_boop.py, line 324 [r3] (raw file):
This if/elif statement can be simplified to a dictionary.


Comments from Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants