Code Review #2 (Spenser Haddad)

[SpenserHaddad]

General

• You may find it useful to have a .gitignore file at the root level of the repository so build- or user-specific files are not included for everyone else. For example, the __pycache__ and .DS_Store directories are machine-specific, so the repo doesn't need to include them.

• Your Archives directory could safely be stored using a git branch, or even as a previous commit.

Demo

• You may want to cite the source for video.py in the docstring.
• There's not much reason to keep previous/test versions of files labelled as such in Git: just overwrite them with a commit.
• Similarly, mcuh of the commented code could be removed, but saved in Git. Some files have so much commented code that it's hard to find and read the actual parts.
• Do not include API Keys in Git! Use a local file that you don't check in, or environment variables. See: Demo/Final_Andriod.py (also there's a typo, but that's less severe)
  • The API key doesn't seemed to be used anywhere, and was replaced with local files elsewhere. Still, it would be good to remove the key from the code.
• When calculating frames per second, you wait until the code is complete and calculate the total number of frames completed in the total time. This may be inaccurate if there are slowdowns in certain cases. You may get more accurate results if you record the time elapsed between each frame and use that to create an average fps at the end.

Python

• Try to use PEP8's guidelines for module (file) names.
• Avoid using reserved keywords as variable names (file, id, etc.) In particular, change instances of: file = open(foo, 'r') # Do something... file.close() to the more Pythonic: with open(foo) as f: # Do something
• Use for idx, val in enumerate(some_iterable) instead of for idx in range(len(some_iterable
• Use more functions so your main loops aren't so large and heavily indented.
• Cut down on the number of magic numbers in your code (ex. if (flag % 10 == 0) is not very clear on a first read).

Arduino (C/C++)

• You can use switch statements instead of if (currChar == 'a') for clarity.
• Overall, the code is clean and straightforward.

Examples + Tutorials

• These were very helpful for understanding your demo code, and are good examples in general. Thank you for including them!

[philyuan]

Thanks for the comments!

We cleaned up the code to also include your advice including removing reserved keywords. The API keys were a mistake (which GitHub promptly detects and sends a warning email conveniently enough) and were immediately regenerated, but thanks for noticing!