Turning in MP4 #1
Conversation
| @@ -1,3 +1,33 @@ | |||
| # InteractiveProgramming | |||
| # Interactive Programming | |||
| import random, pygame, sys, pygame.font | ||
| from pygame.locals import * | ||
|
|
||
| navyblue = (60,60,100) |
There was a problem hiding this comment.
Avoid global variables when you can. It seems like you could make these class attributes.
There was a problem hiding this comment.
A special case of a variable that doesn't actually vary – that's used as a constant. Python-the-languge doesn't distinguish between constants and other variables, but Python-as-used-by-humans does. The convention for a constant is all uppercase NAVY_BLUE.
| @@ -0,0 +1,34 @@ | |||
| import random, pygame, sys, pygame.font | |||
There was a problem hiding this comment.
It's good practice to have a header comment at the top of every file to explain what this file does. That way when people are cruising through all the files they can easily read a summary of the code.
| self.rect.centerx = self.x + (self.image.get_width()/2) | ||
| self.rect.centery = self.y + (self.image.get_height()/2) | ||
|
|
||
| #figure out direction to fruit to move based on spawn position (left or right) |
| self.rect.centery = self.y + (self.image.get_height()/2) | ||
|
|
||
| #figure out direction to fruit to move based on spawn position (left or right) | ||
| border = screen.get_width()/2 |
There was a problem hiding this comment.
The rest of this init function could almost be it's own method that init calls. It's determining how the fruit will move so it's can be sectioned off in a comprehensible manner and there are only two class attributes that are created in 15 lines of code.
| self.rect.centerx = x + (self.image.get_width()/2) | ||
| self.rect.centery = y + (self.image.get_height()/2) | ||
|
|
||
| def toss(self, time_passed): |
There was a problem hiding this comment.
There's a lot going on in this function that result in tossing. While the name of the function kind of explains what the function does, it might be worth having a docstring to explain the specifics.
| add_y = .7*time_passed | ||
| self.y += add_y | ||
|
|
||
| class Apple(Fruit): |
There was a problem hiding this comment.
These fruit classes/half fruit classes could have been one class with an extra variable in the init call that determines the name of the file. It would have brought you down to 2 more classes rather than 9 more classes.
| @@ -0,0 +1,246 @@ | |||
| import random, pygame, sys, math | |||
There was a problem hiding this comment.
Really your master branch should only have the files that you need to run the program successfully. These redundant or unneeded files should either be on a separate 'dev' branch or not present at all. It took me a while to figure out why you had redundant code :)
There was a problem hiding this comment.
I saw below that you had a test folder. This file could go there or have it's own or be deleted. Just something to show that it's not part of the main thing beyond just your README
| from all_fruits import * | ||
| from Scoring import * | ||
|
|
||
| # Hand Tracking Parameters |
There was a problem hiding this comment.
All of the functions in this file should be in a 'game' class. Having it freeform like this makes me sad.
| import imutils | ||
| import cv2 | ||
| import pygame | ||
|
|
There was a problem hiding this comment.
This should be wrapped up in a function at the very least
| class Fruit(pygame.sprite.Sprite): | ||
| def __init__(self, screen, image_name): | ||
| # Call the parent class (Sprite) constructor | ||
| pygame.sprite.Sprite.__init__(self) |
There was a problem hiding this comment.
You can use super().__init__(self) instead of pygame.sprite.Sprite.__init__(self). This makes the code less fragile (you can can change class Fruit(pygame.sprite.Sprite) to class Fruit(MySprite):, where MySprite is some intermediate subclass of pygame.sprite.Sprite, without needing to search Fruit's methods for occurrences of pygame.sprite.Sprite).
This also allows you to eliminate the comment, since the code is now self-documenting.
| max_angle = math.atan(max_height/distance_from_border) | ||
|
|
||
| self.angle = random.randint(int(min_angle), int(max_angle)) | ||
| except: |
There was a problem hiding this comment.
Handle a specific exception.
In this specific case, use math.atan2 instead of math.atan. Then there's no chance of an exception, and there's no numerical instability when distance_from_border is close, but not equal, to 0.0.
|
|
||
| y_speed = random.randint(30,43) | ||
| add_y = math.cos(self.time) *y_speed | ||
| self.y -= add_y |
There was a problem hiding this comment.
Going beyond: there's a lot of code that does something to x, and then the same thing to y. You can use numpy vectors to reduce the redundancy (at the cost of using a more complicated data structure).
| self.screen.blit(self.image, (self.x, self.y)) | ||
|
|
||
| def checkCollision(self, other): | ||
| # returns True or False if apple has collided with other object |
There was a problem hiding this comment.
Since this comment describes the specification of the method, it could actually be a docstring. Advantages: signals to the reader that this describes what the code does, not how it works; will show up in generated documentation.
No description provided.