Skip to content

Conversation

frantonlin
Copy link

@frantonlin frantonlin commented May 5, 2016

They didn't create a pull request to submit this project, so I'm doing it to be able to create comments on lines in reviewable.


This change is Reviewable

laurengulland and others added 30 commits March 5, 2016 21:23
…ltiple rooms, different functionality, and Room object for initialization.
…xed part of hover functionality. Restructured.
…play (incorrect) colors. Hover still doesnt really work.
…te, and most boxes in heaviness of sleep are showing up white too.
@frantonlin
Copy link
Author

You should clean up and remove test files before submitting. Also, you didn't submit a pull request to officially turn in this mini project.


Reviewed 12 of 12 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.


InteractiveProgrammingProjectWriteup.pdf, line 0 [r1] (raw file):
Looks good!


WH_ShapeMap.py, line 6 [r1] (raw file):
Remember to include a header comment with the purpose of the specific file and that you are the writers of this code.


WH_ShapeMap.py, line 28 [r1] (raw file):
You should include docstrings for your classes and functions. That way, it's easy to understand when you look back or other people look at your code.


WH_ShapeMap.py, line 43 [r1] (raw file):
Docstrings! It's fairly difficult to tell how your coordinate system works at first glance. If this giant if statement just hard codes room numbers (minus the first digit) to positions, it might have been more intuitive and efficient to have created a dictionary of room numbers to coordinates.


WH_ShapeMap.py, line 113 [r1] (raw file):
This may also have been more intuitive as a dictionary, as you wouldn't need to remember which number is associated with each property. Also, you initialize the colors list as integers, but then you later replace the integers with strings, which is a little confusing from a style standpoint.


WH_ShapeMap.py, line 169 [r1] (raw file):
Lots of repetitive code here, seems like a for loop might've saved you some typing.


WH_ShapeMap.py, line 231 [r1] (raw file):
Definitely could've used a loop here and added each map figure to a list.


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