Skip to content

Code review #10: No emailed code questions

[Update #1: below is added in response to Hersh’s comment]
Developers have this:

Yet developers are (should be) willing to spend time explaining the code to new developers. Including emails and chats conversations, in-person conversations, etc.

I don’t insist on arbitrary mounds of (unnecessary) comments. Rather I want minimal comments (once again read this previous post about commenting the “why”)
[/update #1]

However when the questions do come up, developers I work with are not allowed to ask in person, NOR chat, NOR email me questions about specific pieces of code.

Instead, they must ask their questions as comments in the code. I will reply as a comment as well.

For example from production code:

// TO_KOSTYA why are we getting nvr records?
// TO_PAT this is because we create FILTER_BY_CATEGORY message end point records, for every broadcast envelope. So a message endpoint record is created for every endpoint, even if a message never goes in there. Such records are never posted to external services, which results in the externalEntityStatus to be returned as 'nvr'.
// TO_KOSTYA : we need to filter at the server. Those records are essentially a server implementation detail that clients should not see.
if ($envelopeStatus['externalEntityStatus'] == 'nvr') {

This technique has huge benefits:

  1. Other developers will see the conversation only if they care about the conversation (i.e. they are debugging something that passes through the block of code with the question)
  2. The thinking/reasoning that would normally be lost and buried in the mass of email is preserved for others – including new developers that show up months or years later.
  3. No speculative commenting: Developers writing the same text that they would in a chat or email and in the process are commenting code that is the most troublesome/confusing and needs commenting the most.
  4. If the underlying issue is resolved, then the comment block can be rolled up into a coherent more generic comment.
  5. Decisions about what should NOT be done are also captured right where an uninformed developer is likely to reintroduce a bug

As to my no chat/no in-person/no email rule: I am naturally flexible (we do have to get work done after all) about this with the requirement. However, the person asking the question must:

  • take the chat log and put it as a comment in the code
  • OR create a comment that would have answered their question if they saw the comment.

Notice what is happening, instead of putting developer time into an email exchange, the same effort is being put into the comments:

  • Developer time is unchanged,
  • the text is unchanged,
  • only the location of the conversation is changed.

Posted in code review, self improvement, technical.

2 Responses

Stay in touch with the conversation, subscribe to the RSS feed for comments on this post.

  1. Hersh says

    Hi Patrick,
    Interesting post,
    I would think though your process could be augmented by few things:
    1. A design doc before you code which could be as simple as a picture of a sketch.
    2. A code review before code is pushed into production.
    3. unit tests, I think test cases do a great job of conveying meaning.
    The process you described seems most appropriate for scenarios with existing undocumented code.

    • patrick says

      @Hersh — I respond in part with a post edit to clarify that this method is to address the “I don’t have time for comments” issue.

      To the specific points:
      1. Design documents before you code – this post is really about when the code is growing and expanding. Design documents need to be maintained as the code changes. Certainly highly level system architecture sketches have their place. This post is addressing the more granular level of commenting

      2. code reviews: this is what we are talking about 🙂 A developer looks at the code and has a question… only instead of the question being spoken or emailed it is delivered as a code comment

      3. unit tests – nice but don’t always happen especially when large chunks of new code is written and the design and everything else is in flux.

      Lastly, note that this post is not about additional effort and developer work. This post is about keeping the work the same but in a different location.

Some HTML is OK

or, reply to this post via trackback.