Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 6 Next »

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
  • 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?

Jan 20, 2016

Chapter 6: Objects and Data Structures

  • Law of Demeter
    • "objects that you created" is Java-specific
    • distinguishing between objects that you've created and those that you depend on
    • abstraction layers
    • in some places, it says no more than one period.
    • Train wrecks
      • distinguishing between breaking abstractions through objects, and
      • accessing data models directly
  • Objects versus Data Structures
    • Our django models have become hybrids
    • What's the right pattern of having separation of concerns between persistence and the object-level abstraction?
  • Should we create a new business logic layer on top of our models?
  • Too many layers is not advantageous: API view -> Utils function -> .... eventually a Model
  • Trade-off between making use of the framework and having abstraction layers
  • If we think about the edx-platform codebase history, it seems we add more behaviors overtime, but no data structures.
    • This implies perhaps we should think more procedural, rather than OO.

Chapter 7: Error Handling

  • Using exceptions rather than error codes
    •  No brainer.
  • Throwing exceptions instead of returning None
    • We do this in our codebase.
    • It's fine as long as we document it - we don't have a result for you.
    • We do like his suggestion of having a special case object.
  • Using django framework conventions
    • ObjectDoesNotExist exception
    • get_or_none - returns None
    • first - return None
  • In Java, exceptions are expensive, so they try to avoid them.
    • But in Python, that's not the case.
  • For example, getattr, just catches the exception and returns False/True.
    • But if catching the exception is acceptable for code, then go fo it.
  • By returning error codes, the callee can be assured that the caller will handle all cases.
    • But then too many if statements.
    • And probably end up wrapping it anyway, and throw an exception.
    • If you make a mistake, and completely miss a case, then would get unnoticed.
      • In some other languages, however, you have to pull out your object explicitly.
    • hasattr - in case the object raises an exception other than AttributeError, later version of Python now catches it and returns False.  Before, the exception was passed through.
  • API wrapper mixin makes it easy to translate View exceptions to formatted HTTP error return objects.
  • Some languages have @Nullable decorators to explicitly specify which functions have chosen to return Null.
  • Passing Null around
    • In some cases, we do want to do this - for example, Null values in Address fields, as opposed to empty strings.
    • One way of thinking about this for Python: passing None for optional parameters is fine, but for non-optional parameters should be seriously vetted.
    • "?." is not yet accepted into Python.

Next time: Chapters 8 and 9.


  • No labels