Book Clubbers: Michael Katz (Deactivated), Christopher Lee (Deactivated), Cliff Dyer (Deactivated), akivaR (Deactivated), Andy Armstrong (Deactivated), ChristinaR (Deactivated), Eric Fischer (Deactivated), Robert Raposa, Renzo Lucioni (Deactivated), ClintonB (Deactivated), Adam Palay (Deactivated), Nimisha Asthagiri (Deactivated)
Jan 6, 2016
Thoughts on organizing the book club
- Case study of actual edX code
- Go through each chapter and decide on what applies to edX
- Overall thoughts on the book - writing, content, approach
Book Thoughts
- We like that he acknowledges that there are multiple perspectives and multiple schools of thought
- We like that he takes something complicated and makes it simpler
Chapter 3: Functions
- Does smaller (and more) functions actually make it more readable?
- In python
- By the way, function calls don't actually get optimized away.
- Performance issues should be exceptional cases - and should be commented as such.
- Is Python's coding practice counter to these rules?
- As a team that does a lot of code reviews, code readability is very useful.
- Functions should only do 1 thing
- Triad rule of number of parameters?
- Python does have keyword arguments that helps mitigate this.
- But JS doesn't.
- But too many arguments means more combinations for testing
- And also, we tend to have arguments that are mutually exclusive of another.
- We may want to still flag this situation and consider why.
- Step-down rule
- Boy Scout rule
- Clean up while updating code.
- This is very difficult to do because it takes us down a long path.
- How to encourage and make this happen at edX?
- Be stricter in code reviews.
- Have separate commits for refactoring.
- Sometimes it's difficult because of merge conflicts.
- Ways of deprecating - come back to it.
- Duck-typing in Python?
- Example: Argument can be a String or an Object.
- PyContracts
- Functions that are both getters-and-setters
- Side Effects
- Error Handling
- all error handling in a separate function? really?
- case study: views.py versus api.py separation of concerns.
- api.py really needed?
- If there are no in-process requirement for it, then just exclude it?
- If people start using api.py and we want to separate into an IDA, it makes it difficult.
- If we don't create an api.py initially, we can create it later when it's actually required.
- For an app with many views, you can use a views folder with multiple modules.
edX Takeaways
- Is there somewhere we can point to for best practices?
Jan 13, 2016
Chapter 4: Comments
- Comments do get out of date. So commenting on things that aren't obvious makes sense.
- Do we have an obligation when it comes to comments because of the open source nature of the platform
- API Docs
- pylint - docstring requirements
- explicit disabling locally versus updating pylint to no longer require docstrings
- but if explicitly disabled locally, that can also get stale
- Why do docstrings have to be multi-line?
- Takes up a lot of vertical space
- Rationale: Code diffs are clearer? (similar to the trailing comma guideline)
- Petition against Ned Batchelder (Deactivated)?
- Coding convention: triple double quotes
- Ambiguity of parameters
- "Could be a string or a list"
- Duck typing
- JSON is mildly typed
- API bindings
- DRF-Swagger will automatically create docstrings
- Now that DRF is upgraded, we can look into this.
- ClintonB (Deactivated) will send a link to how they do this for Course Discovery
- Can create common DRF Serializer fields - for example, for OpaqueKeys.
- TODO comments
- Have corresponding JIRA tickets
- Distinguish between
- short-term TODOs (address before merging the PR)
- long-term TODOs (with JIRA tickets)
- TODOs for code improvements - may not need tickets
- lint-checker for TODO to contain ticket numbers
- When to use comments and when not to seemed conflicted
- All comments can get stale
- So only write comments that are really necessary
- In place of comments
- use intermediate, well named, local variables
- for conditionals
- for long lines
- But when there are local variables, you expect it to be used multiple times.
- better method names
- use intermediate, well named, local variables
- List comprehensions that are 5 lines should probably be for loops instead - for readability.
- The python ternary operator reads much better since it's in one line.
- But some developers don't like them as much.
- "x or y" OR "x if x else y"
Chapter 5: Formatting
- Python is very explicit
- Dependent functions
- In test files, we've generally been putting the helper functions on top (which is reverse from his recommendations)
- But for other classes, we generally use the top-down approach.
- For xCode
- File hierarchy on disk is different from hierarchy in the IDE
- So needs to be manually updated
- Agree: Extra whitespace brings attention to wrong things
- Like: Breaking indentation always returning back to the indented form
- Agree: If statements not having braces
- In JS, variable declarations always on top because of scoping rules, but is it helpful for readability?