Project Submission by iblancett · Pull Request #4 · sd17fall/InteractiveProgramming

iblancett · 2017-10-30T05:34:04Z

No description provided.

…cs.py

…ramming

…to implement icon images

…l need to clear screen so text goes away after not over icon.

…ramming

Elepert

Good job! Your overall code looks clean. Your documentation was really thorough. The biggest improvement you could make is reorganizing your files to make some functions into class methods rather than free floating and organize some free floating code into functions/methods.

Elepert · 2017-12-04T02:44:45Z

Source/courses.py

+# Author: Isa Blancett
+# Project: Interactive Programming
+# Date: 10.30.2017
+# Description: Class for reading in a csv file containing course information and


Good job with the header comments on the files!

Elepert · 2017-12-04T02:46:17Z

Source/courses.py

+class Courses(object):
+    """ class for the different course options the user can choose from
+
+    contains:


Good job explaining what all of these are. An extra step you could do is specifically write what type these will end up being (ie: string, int, etc..). It's not necessary for this project, but it's good practice when you're making data structures like these.

Elepert · 2017-12-04T02:51:55Z

Source/gameplay.py

@@ -0,0 +1,124 @@
+# Author: Isa Blancett


Good job with the unit testing!

Elepert · 2017-12-04T02:53:06Z

Source/main.py

+from ending import ending
+from environment import *
+
+# INITIALIZE GAME


The code below should be in a function or a class. It's always good practice to wrap things up and not have them free floating in a file.

Elepert · 2017-12-04T02:54:05Z

Source/environment.py

+import pygame
+import os
+import sys
+


I would put all the functions below into a class for the screen. You're changing an object in your game essentially which is when classes are really nice :)

Elepert · 2017-12-04T02:54:45Z

Source/ending.py

+
+import pygame
+import sys
+


I wonder if these functions could be put into the GamePlay class. They seem to be important parts of it. Rather than importing into the file, adding them to the class makes more sense

osteele · 2017-12-05T20:08:33Z

Source/courses.py

+    """
+
+    new_course = Courses()
+    new_course.setup(config)


Consider changing Courses.setup to Courses.__init__, and changing these two lines to new_course == Courses(config).

The principles are:

Code that requires the client to perform a sequence of calls in order is fragile.

Reduce the amount of time that an object is in an unusable state. It doesn't make sense to use new_course after Course() and before new_course.setup(…), so shrink the period during which the class is exposed in this state.

Make it impossible (or harder) for the client of a class or module to mis-use it.

This principles all come to the same thing here; I'm listing them as separate principles because they're all good design principles, and they don't always apply to the same situations.

In this case, replacing Courses.setup by Courses.__init__ allows you to remove create_course entirely, and replace calls to create_course(config) by Courses(config).

osteele · 2017-12-05T20:08:51Z

Source/courses.py

+
+    new_course = Courses()
+    new_course.setup(config)
+    return(new_course)


return is a statement, not a function; the parentheses aren't necessary (or idiomatic).

osteele · 2017-12-05T20:09:28Z

Source/courses.py

+            row['dependencies'] = row['dependencies'].split()
+        else:
+            row['dependencies'] = []
+        new_course = (create_course(row))


outer parens aren't necessary

osteele · 2017-12-05T20:10:19Z

Source/ending.py

+    '''Turns screen a color by filling in with rectangles.'''
+    for i in range(700):
+        pygame.draw.rect(screen, color, (0,i,1000,i+1))
+        i+=1


The i+=1 line doesn't do anything, since the next time through the loop resets i

osteele · 2017-12-05T20:10:57Z

Source/ending.py

+import pygame
+import sys
+
+def screenScroll(screen, color):


The Python convention is screen_scroll.

osteele · 2017-12-05T20:11:51Z

Source/ending.py

+        screenScroll(screen, (0,0,0))
+        screen.blit(font50.render("Looks like your portfolio", 1, (255,255,255)), (300, 250))
+        screen.blit(font50.render("could use some improvement", 1, (255,255,255)), (280, 350))
+        Done = False


The convention for variables is lowercase.

osteele · 2017-12-05T20:12:54Z

Source/ending.py

+                if event.type == pygame.QUIT:
+                    Done = True
+            for i in range(len(levels)):
+                screen.blit(font20.render(levels[i], 1, (200,200,200)), (100, 450+i*25))


You can also use:

for i, level in enumerate(levels): screen.blit(font20.render(level, 1, (200,200,200)), (100, 450+i*25))

osteele · 2017-12-05T20:13:35Z

Source/ending.py

+            for i in range(len(levels)):
+                screen.blit(font20.render(levels[i], 1, (200,200,200)), (100, 450+i*25))
+            pygame.display.flip()
+    if victory == 1:


It looks like victory is used a boolean? If so, prefer the boolean type: True/False, instead of 1/0. This is clearer in intent.

osteele · 2017-12-05T20:17:26Z

Source/ending.py

+def determine(victory, levels):
+    '''Determines ending.
+    Victory is value 0, 1, or 2. 0 is bad, 2 is perfect.
+    Levels is a list of strings. Print out with for loop'''


For code that documents itself without as many explicit docstrings and comments, consider:

BAD_OUTCOME = 'bad' MIDDLE_OUTCOME = 'middle' PERFECT_OUTCOME = 'perfect' … if victory == MIDDLE_OUTCOME: …

This has the advantages that it's easy to see the intent each place the value is used, without having to find the nearest comment that describes it; that you don't need to repeat comments in each function that generates or uses these values (admittedly, not that many places in this example); that printing out victory is more helpful even if you've forgotten what represents what; and that you're better protected against a certain class of errors, for example using 4 as a victory value.

osteele · 2017-12-05T20:19:12Z

Source/environment.py

+    FMatSci_Icon = pygame.transform.scale(SMatSci_Icon, (108, 100))
+    SMechProto_Icon = pygame.image.load('Source/Course_Icons/mech.png')
+    FMechProto_Icon = pygame.transform.scale(SMechProto_Icon, (108, 100))
+    #basic background


To take this to the next level, consider replacing these separate variables by a list.

osteele · 2017-12-05T20:20:03Z

Source/environment.py

+    '''This function looks for the click of a mouse and queues an event based on where the mouse is'''
+    (x,y) = pygame.mouse.get_pos()
+    clicked_path = 'Source/Course_Icons/%s_clicked.png'
+    if y >= 550 and y<= 655 and any(pygame.mouse.get_pressed()):


Python tip: you can also use if 550 <= y <= 655

osteele · 2017-12-05T20:20:39Z

Source/environment.py

+    (x,y) = pygame.mouse.get_pos()
+    clicked_path = 'Source/Course_Icons/%s_clicked.png'
+    if y >= 550 and y<= 655 and any(pygame.mouse.get_pressed()):
+        if x >= 51 and x<= 159 and 'bees' not in order:


Where do these numbers come from? Can they come from a list of entities and their locations?

sokuno222 and others added 30 commits October 18, 2017 20:26

Added class defintions for course and req. Req is incomplete.

2c87b85

Added import script

24678e7

Added ideation and source folders

a168e80

progress on algos -- nothing working yet

42abeb2

Updating graphics window script. Added incomplete mouse pressed event

8730859

Courses can now be made

d959455

Commiting to resolve pull errors. Added mouse_pressed event to graphi…

f4556da

…cs.py

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

dba8016

…ramming

Made course icons

8976520

started game control

5118db2

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

2d2ea2d

…ramming

Started setting up background function for graphics.py to call. Need …

c15d2ea

…to implement icon images

Made some level icons

4933c21

Changed files to incorperate icon images and mouseOver function. Stil…

e4c2e94

…l need to clear screen so text goes away after not over icon.

Algo implemented for game control

afea157

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

a786ea4

…ramming

Something didn't push

d0b42b6

Deleted unused scripts

96e4cd4

Added some doctests to Gameplay and debugged

b133a20

Debugged some more, added ending verdict and more doctests

03ad23a

added communication between gameplay and main

459d704

Rearranged environment images

f273cc2

Setup portfolio class

31a7cdb

changed file paths to work on Mac

163590c

Moved rendered text to setUpE. Course Image loading not working

5b05cd1

Fixed rendering error in setUpE

30641b9

Readded rendering font to mouseover function

2c3120d

Fixing text overwriting problem

07ab946

Fixed text writing over itself and size

bd1937a

Moved text rendering to setUpE, Made setUpE run only once.

e183459

iblancett and others added 17 commits October 29, 2017 22:41

Added headers

6894572

Added course descriptions

4130462

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

5863d32

…ramming

revised project reflection

f63593d

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

8f2efc1

…ramming

update on the portfolio image

646e64c

Changing color

8bfb444

Fixed portfolio function

6f88e50

Update on README

62e75c2

Created ending.py to make text ending

4b50ce2

Added ending files.

9dcf674

Merge branch 'master' of https://github.com/iblancett/InteractiveProg…

245d255

…ramming

Added call to ending function

340cbe8

Fixed ending

b96b29e

Got endings working, window no longer quits

4549dc2

Fixed typo

e773ac0

Fixed icon descriptions to match the correct icons

e6a9e17

Elepert reviewed Dec 4, 2017

View reviewed changes

osteele reviewed Dec 5, 2017

View reviewed changes

Source/ending.py

import pygame

import sys

def screenScroll(screen, color):

Copy link

Contributor

osteele Dec 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Python convention is screen_scroll.

osteele reviewed Dec 5, 2017

View reviewed changes

Conversation

iblancett commented Oct 30, 2017

Uh oh!

Elepert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants