One of the most active threads on reddit’s programming section right now discusses things people look for when reviewing someone else’s code; the article being discussed treats this as a great interview question and points to things like algorithm choices and object-oriented design as good responses. While these are important considerations, I’ve found I tend to make snap judgments long before I get to that level of analysis, and they’re almost always based on one key factor: documentation.
Of course, I have the luxury of mostly reviewing Python code, and Python makes it ridiculously easy to embed useful documentation. What’s more, automated API-documentation generators don’t seem to be as popular in Python as they are in some other languages, which means the lazy attitude of “a big list of function names and argument signatures should be all the documentation you need” thankfully is not prevalent in Python.
So when I’m looking at someone’s code, the first thing I typically notice is how well it’s documented. And I’ve found that the presence of useful documentation is quite often an indicator of a particular developer’s quality; in my experience, it’s rare to find bad code with good documentation. So I’d like to run through a few documentation-related things that I usually look for when I’m reading a piece of code for the first time.
Python, like a few other languages (e.g., members of the Lisp family), actually offers two ways to document any given piece of code. One is by inserting comments — prefixed, in Python’s case, with the # marker — at relevant places throughout the code. The other is via docstrings, which allow longer, free-form documentation to be associated with (typically — in theory, any Python object can have a docstring) a particular function, class or module. This is interesting for two reasons:
In certain circles, comments in code are frowned upon, if not actively discouraged, because the presence of a comment is seen as an indicator that the code by itself was not clear enough to communicate its purpose and workings to the reader. Thus, the need to provide explanatory comments is occasionally seen as a weakness which should be fixed by writing better code.
I’d really rather not get into that debate here, but I do think that Python’s solution of offloading documentation into a tailor-made language feature rather than overloading the comment mechanism draws a clear distinction between documentation which explains what a piece of code does and occasionally how it does it, and documentation which explains why a particular technique was chosen. In my opinion, the first two items — “what” and “how” — generally belong in docstrings, and the third — “why” — typically should either be made clear by the code or explained in a comment.
Since this is a somewhat nebulous thing to say, let me take a real-world example. In django-registration, a new, inactive user account has an activation key associated with it; this key is a salted SHA1 hash, and to activate the account the new user must visit a page with the activation key in the URL. The method which generates this key explains what is happening and how:
def create_profile(self, user): """ Create a ``RegistrationProfile`` for a given ``User``, and return the ``RegistrationProfile``. The activation key for the ``RegistrationProfile`` will be a SHA1 hash, generated from a combination of the ``User``'s username and a random salt. """
But the default URL pattern which routes to the activation view — and thus which needs to match the activation key to pass it as an argument to the view — looks like this:
url(r'^activate/(?P<activation_key>\w+)/$', activate, name='registration_activate'),
This is a bit strange. The activation key, as we already know, is a SHA1 hash, which serializes to a 40-digit hexadecimal number. The regular expression which matches an activation key, then, is [a-fA-F0-9]{40}, but the regular expression used in this URL pattern is \w+, which will match any sequence of word characters. There’s a good reason for this: if the pattern only matched things that look exactly like SHA1 hashes, it could easily result in failed matches for things like incorrect copying and pasting from an activation email, returning a “404 not found” instead of a more useful error message saying that the activation key was invalid.
Now, this is a detail that really doesn’t need to be mentioned in high-level documentation, but someone casually reading the code could easily spot this and wonder if it’s a bug and there’s not really any way to rewrite the code so that the reasoning behind it will be clear. Thus, an explanation of why this URL pattern uses the regex it does is provided in a comment directly above the pattern. Here’s the full pattern with the comment:
# Activation keys get matched by \w+ instead of the more specific # [a-fA-F0-9]{40} because a bad activation key should still get to the view; # that way it can return a sensible "invalid key" message instead of a # confusing 404. url(r'^activate/(?P<activation_key>\w+)/$', activate, name='registration_activate'),
Distinguishing between these questions and the places where they should generally be answered — “what” and “how” in docstrings, and “why” in comments — is a big thing that I look for when reading code. Also, failure to distinguish between docstrings and comments, or recognizing only one or the other, is something that’ll get you marked down in my book; I’ve often griped, for example, about Ohloh, which treats documentation as a metric for judging code quality but refuses to recognize docstrings in Python code as documentation.
Before I go any further, let me just get this out of the way: I’m really, really, really anal about documentation. Those of you who know me or who’ve read through some of my code can attest to the fact that I try quite hard to practice what I preach, sometimes taking it to a ridiculous level. I don’t expect everyone to take documentation to the extremes I do, but I do expect good code to include good documentation. Python has a style guide for docstrings which includes some good, sensible recommendations, and following that can take you quite a long way. Let me add a few tips:
get_absolute_url(), and the purpose of this method is well-understood; it’s a convention of the Django framework and is documented as such, so constantly repeating a docstring which says “Return the public URL for viewing this object” is pointless. On the other hand, if you’re writing a model and you override the save() method to provide custom behavior, you should give that a docstring to explain what it’s doing that varies from the standard behavior.
And if you want to go the hardcore route, you can try something I find myself doing fairly often and which, in a slightly drunken chat with Michael, Eric and others at PyCon, I dubbed “Documentation-Driven Development”. You may already be familiar with the idea of “Test-Driven Development” or “TDD”, where you write a unit test for a new piece of code before you write the code itself, in which case you can guess what I’m getting at: I typically write the docstring first, and then write the code to match it.
Of course, it’s not always that easy, and typically I start with just a short outline and flesh it out as I go, but I’ve found this to be an incredible aid to producing usefully-documented code. As an added bonus, it meshes neatly with a couple more aphorisms from the Zen of Python:
If the implementation is hard to explain, it’s a bad idea.
If the implementation is easy to explain, it may be a good idea.
If I can’t write a simple, short docstring describing what a piece of code will do, it’s almost always a bad idea to try to write it without first putting some more thought into the design. And since it’s much easier (from a psychological perspective) to scrap a fledgling docstring and start over than it is to throw out a piece of code, this has saved me quite a lot of time and effort; instead of realizing after the fact that I’d written some code badly, or written some code I shouldn’t have, I’ve found myself realizing ahead of time that I’m describing code I shouldn’t write at all, and stopping.
I don’t think this should be the only way to write code (I don’t always write this way myself), and I don’t think anyone should be forced to do it, but I have found it to be quite useful; if you’re feeling adventurous, give it a try sometime and see if it works well for you, too. If nothing else, evidence of obsessive documentation practices will win you major brownie points if I ever have to review your code.
There’s one more thing that really stands out, even at a glance, in well-documented Python code: functional examples. Just including an example or two of how to use a piece of code goes a long way, but Python can help you get even more mileage out of that documentation. The secret lies in Python’s “doctest” module, which can treat examples in your docstrings as unit tests for your code.
I don’t use doctest nearly as often as I ought to, but here’s a fairly complete example from a little library I wrote for working with HTML and CSS color codes (also: this module, which has about 240 lines of actual Python code in a file over 800 lines long, is a good example of what I meant when I mentioned I occasionally take documentation to “ridiculous” levels):
def rgb_to_hex(rgb_triplet): """ Convert a 3-tuple of integers, suitable for use in an ``rgb()`` color triplet, to a normalized hexadecimal value for that color. Examples: >>> rgb_to_hex((255, 255, 255)) '#ffffff' >>> rgb_to_hex((0, 0, 128)) '#000080' >>> rgb_to_hex((33, 56, 192)) '#2138c0' """ return '#%02x%02x%02x' % rgb_triplet
At first glance this looks like a perfectly normal docstring: it explains what the function does, and provides some examples to illustrate its use. But lurking down at the bottom of the module is this little piece of code:
if __name__ == '__main__': import doctest doctest.testmod()
When executed “standalone” as a Python script, this module will invoke doctest which will, in turn, scan the docstrings looking for bits that resemble Python interpreter sessions, like the examples in the docstring above. It will then execute the lines of code in those examples and verify that the results match what’s in the docstrings. In other words, the documentation also serves as a verifiable functional specification for the code, because the embedded documentation can be executed as a unit-test suite.
If you’re using Django, keep in mind that the Django test framework can locate and use doctest-based tests in your code. Quite a bit of Django’s own test suite is written this way, and doctest is used to some advantage to produce files which serve as both API examples and unit tests; for example, this file in Django’s test suite is located by the test system and executed as a set of unit tests, but also can be parsed by the documentation builder to produce this example in the online docs.
While the obsessive levels to which I take documentation may well be unique to me, looking at documentation as an indicator of code quality (and, hence, developer quality) almost certainly isn’t. So if you aren’t taking advantage of the tools Python gives you to produce well-documented code, start now. And if you’re using Django, you really should start documenting your code, because Django provides some useful helpers for writing and accessing documentation.
Comments for this entry are closed. If you'd like to share your thoughts on this entry with me, please contact me directly.
I also have some django models that implement unicode. This was done originally so I could give them a human readable representation in the auto generated admin interface. However, I noticed the other day that my IDE (Komodo) has taken to using these as the value for hover-over tooltips. Damn handy, I can tell you.
It helps maintain code, and therefore counts as documentation :)
Great post, thanks!
I would clarify that the “how” portion should be split, IMHO. The “how do I interact with this” should go in the docstrings while the “how is this implemented” is better left relegated to comments. Thus, you might have a docstring that details parameters and return values (how to interact with the documentee), which is helpful from the command-line where you use a help() call. Doctests also fall nicely into this “how do I interact with it” classification.
On the other hand, the internal “how does this work” (“balance the red-black tree”; “iterate over them in primary-key order for faster access”) type comments aren’t useful in a help() call and thus I’d say they work better in a comment.
Just my $0.02, ever devaluating against international currency markets and buying less and less gasoline.
-tim
Excellent post! I’m pleased that you brought up the “Documentation Driven Development” philosophy. That really was an a-ha moment for me. (Even among all the alcohol.) It’s a wonderful practice, and I’m glad that you introduced me to the concept. Although, I have to become more disciplined in the practice.
@Empty: “documentation driven development” is called Literate Programming and is around for a really long time, already.
@ak: not quite, no, because LP is much broader in scope and involves highly-specialized file formats and tools to allow, e.g., TeX-formatted prose to be interspersed into a program’s source code and maintain links between segments of prose and segments of code.
What Python offers through its docstrings is much less ambitious than LP, but does have some advantages:
@James Bennett: there is more than one approach than Don-Knuth-WEB/CWEB-style literate programming. Look up a definition of literate programming: it basically says that writing programs should be approached like writing a piece of literature, and therefore the programmer is encouraged to closely couple documentation with the source code. WEB/CWEB, in this context, are merely a tool to support this process, but not a requirement or The One True Way(tm) of any kind.
Another way of explaining the doc strings and comments split is doc strings are for documenting the interface and comments are for documenting the implementation. If you inherit from an abstract base class or declare you implement an interface, then you should not have to override existing doc strings, but you should use comments to explain the details of your implementation. ABCs and Interfaces are also great for creating DRY documentation - although Interfaces are slightly nicer since they are consumer-centric (e.g. they don’t contain self in a method signature) so you can cut-n-paste them directly into your code when you are using an API.
Or another way of explaining it is “what do I need to know to use an object” is for doc strings, and “what do I need to know to develop a class” is for comments.
I think that when we look back in 20 years, and documentation-driven development (or better yet, doctest-driven development) is a serious and developed methodology, people like James will be seen as visionaries. It makes so much sense with the advent of doctests that this be the next step.
However just as with TDD, it’s a discipline that’s going to require some serious…well…discipline.