Upgrade to Pro — share decks privately, control downloads, hide ads and more …

//TODO: Write A Better Comment

//TODO: Write A Better Comment

Presentation at Droidcon NYC 2019 on writing helpful code comments.

Adam McNeilly

August 27, 2019
Tweet

More Decks by Adam McNeilly

Other Decks in Programming

Transcript

  1. "When you need to write a comment, it usually means

    that you have failed to write code that was expressive enough. You should feel a sharp pain in your stomach every time you write a comment." @AdamMc331 #DCNYC19 4
  2. To Provide Additional Insight /** * There is certain functionality

    that we need to be consistent * in all WebViews of our app. * For some URLs, though, we need additional customization so we * can extend this base class accordingly. */ class BaseWebViewClient(...) : WebViewClient @AdamMc331 #DCNYC19 8
  3. To Explain Why We Did Something // The API returns

    the time in seconds // but we need to manipulate it as milliseconds. val timeInMillis = response.time * 1000 @AdamMc331 #DCNYC19 9
  4. To Provide Documentation interface AccountDAO { /** * Inserts an

    account into the database. * * @param[account] The account that we're inserting. * @return The ID of the inserted account. */ fun insert(account: Account): Long } @AdamMc331 #DCNYC19 10
  5. Describe Some Action // We only want active users val

    usersToDisplay = userList.filter { user -> user.isActive } @AdamMc331 #DCNYC19 13
  6. That Action Changed // We only want active users val

    usersToDisplay = userList.filter { user -> user.isActive && user.completedRegistration } @AdamMc331 #DCNYC19 15
  7. Which Is Right? // We only want active users val

    usersToDisplay = userList.filter { user -> user.isActive && user.completedRegistration } @AdamMc331 #DCNYC19 17
  8. Three Types Of Comments • Comments that are unnecessary •

    Comments that are unhelpful @AdamMc331 #DCNYC19 18
  9. Three Types Of Comments • Comments that are unnecessary •

    Comments that are unhelpful • Comments that are helpful @AdamMc331 #DCNYC19 18
  10. Bad: Repeating the code interface AccountDAO { /** * Inserts

    an account into the database. * * @param[account] The account that we're inserting. * @return The ID of the inserted account. */ fun insert(account: Account): Long } @AdamMc331 #DCNYC19 20
  11. Good: Remove what we don't need interface AccountDAO { /**

    * @return The ID of the inserted account. */ fun insert(account: Account): Long } @AdamMc331 #DCNYC19 21
  12. Good: Clarifying Behavior // Saves data to database fun saveData()

    { // ... } Better: More Expressive Code fun saveDataToDatabase() { // ... } @AdamMc331 #DCNYC19 23
  13. Good: Break Up Method fun transferMoney(fromAccount: Account, toAccount: Account, amount:

    Double) { // create withdrawal transaction and remove from fromAccount // ... // create deposit transaction and add from toAccount // ... } Better: Extract Functionality fun transferMoney(fromAccount: Account, toAccount: Account, amount: Double) { withdrawMoney(fromAccount, amount) depositMoney(toAccount, amount) } @AdamMc331 #DCNYC19 24
  14. Now What? • We removed any redundant comments • We

    changed code to avoid comments @AdamMc331 #DCNYC19 25
  15. Now What? • We removed any redundant comments • We

    changed code to avoid comments • How do we ensure the comments we do write are helpful? @AdamMc331 #DCNYC19 25
  16. Comments That Tell Us What /** * A list of

    updated questions to be replaced in our list by an interceptor. */ private val updatedQuestions: MutableMap<Long, Question> = HashMap() @AdamMc331 #DCNYC19 27
  17. Comments That Tell Us Why /** * The `PagedList` class

    from Android is backed by an immutable list. * However, if the user answers a question locally, we want to update the display * without having to fetch the data from the network again. * * To do that, we keep this local cache of questions that the user has * answered during this app session, and later when we are building * the list we can override questions with one from this list, if it exists. * That's determined by the key of this HashMap, which is the question ID. */ private val updatedQuestions: MutableMap<Long, Question> = HashMap() @AdamMc331 #DCNYC19 28
  18. Okay: No Examples class Pokedex { /** * @param[name] The

    name of the Pokemon. */ fun addPokemon(name: String, number: Int) { } } Better: With Examples class Pokedex { /** * @param[name] The name of the Pokemon (Bulbasaur, Ivysaur, Venusaur). */ fun addPokemon(name: String, number: Int) { } } @AdamMc331 #DCNYC19 30
  19. To StackOverflow /** * A ViewPager that cannot be swiped

    by the user, * but only controlled programatically. * * Inspiration: https://stackoverflow.com/a/9650884/3131147 */ class NonSwipeableViewPager( context: Context, attrs: AttributeSet? = null ) : ViewPager(context, attrs) { // ... } @AdamMc331 #DCNYC19 32
  20. To Internal Documentation /** * Implementation of some feature that

    I was asked to build. * * Design/Product Spec: https://confluence.com/some/feature */ class SomeFeatureFragment : Fragment() { // ... } @AdamMc331 #DCNYC19 33
  21. To Reported Issues /** * The carousel library we use

    does not support a * specific functionality that we need. We've extended * this class to modify it ourselves. * * Issue reported: https://github.com/library/issues/1 */ class MyCustomCarousel : Carousel() { // ... } @AdamMc331 #DCNYC19 34
  22. //TODO: Comments If you're not going to do it now,

    create accountability with links to issue trackers. //TODO: Consolidate both of these classes // since we only have one activity now. // AAA-123 class MainActivity : BaseActivity() { // ... } @AdamMc331 #DCNYC19 36
  23. Bad: No Explanation @Deprecated public interface DefaultBehavior { // ...

    } Better: Provide Alternative /** * @deprecated Use {@link AttachedBehavior} instead */ @Deprecated public interface DefaultBehavior { // ... } @AdamMc331 #DCNYC19 38
  24. Reference Definitions • Helps survive refactoring of a definition •

    IDE lets you click into references @AdamMc331 #DCNYC19 42
  25. Reference Definitions • Helps survive refactoring of a definition •

    IDE lets you click into references • Will create links for auto generated documentation @AdamMc331 #DCNYC19 42
  26. Without References /** * Retrieves the primary Type for a

    Pokemon. */ val firstType: Type? get() = currentState.pokemon?.sortedTypes?.firstOrNull() Refactor class name... /** * Retrieves the primary Type for a Pokemon. */ val firstType: PokemonType? get() = currentState.pokemon?.sortedTypes?.firstOrNull() @AdamMc331 #DCNYC19 43
  27. With References /** * Retrieves the primary [Type] for a

    [Pokemon]. */ val firstType: Type? get() = currentState.pokemon?.sortedTypes?.firstOrNull() Refactor class name... /** * Retrieves the primary [PokemonType] for a [Pokemon]. */ val firstType: PokemonType? get() = currentState.pokemon?.sortedTypes?.firstOrNull() @AdamMc331 #DCNYC19 44
  28. Use Consistent Language • When documenting methods that return booleans,

    try to always describe the true condition @AdamMc331 #DCNYC19 45
  29. Use Consistent Language • When documenting methods that return booleans,

    try to always describe the true condition • Don't describe the true response for some methods and the false response for others @AdamMc331 #DCNYC19 45
  30. Inconsistent Documentation /** * @return True if the user has

    signed on within the last 24 hours. */ fun isActive(): Boolean { // ... } /** * @return False if the user is not a staff member for our team. */ fun isStaff(): Boolean { // ... } @AdamMc331 #DCNYC19 46
  31. Consistent Documentation /** * @return True if the user has

    signed on within the last 24 hours. */ fun isActive(): Boolean { // ... } /** * @return True if the user is a staff member of our team. */ fun isStaff(): Boolean { // ... } @AdamMc331 #DCNYC19 47
  32. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments @AdamMc331 #DCNYC19 48
  33. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments • Explain why @AdamMc331 #DCNYC19 48
  34. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments • Explain why • Provide examples @AdamMc331 #DCNYC19 48
  35. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments • Explain why • Provide examples • Give guidance @AdamMc331 #DCNYC19 48
  36. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments • Explain why • Provide examples • Give guidance • Leverage IDE tools @AdamMc331 #DCNYC19 48
  37. Recap • Remove redundant comments • Write expressive code •

    Write helpful comments • Explain why • Provide examples • Give guidance • Leverage IDE tools • Use consistent language @AdamMc331 #DCNYC19 48