Wednesday, June 16, 2010

Book Review: Beautiful Code

I just finished reading Beautiful Code. This programming book is an assortment of stories and examples of well-written code or elegant designs. Each chapter was written by a different author and at a different level of granularity.  My overall opinion of the book was "meh".  I was expecting to see a book full of examples of beautiful code, but instead what I got was a book half full of programmers bragging about how cool their applications are.  I didn't want to read "beautiful applications"; I wanted to read "beautiful code".  Given that, there were some great chapters that included really beautiful code.

  • The section on MapReduce contains the most elegant description of what MapReduce is and how it is used that I've seen.  The code snippets truly are beautiful.
  • The section on "beautiful tests" was quite well written.
  • The section on Fit by Michael Feathers was a fun read.
I found myself skipping whole paragraphs and pages after reading the first two pages of certain chapters because I had no interest in Assembly Hacks or the Solaris Kernel Scheduler - what were beautiful designs, but specific to use-cases I will probably never encounter in real-life.  Fortunately, no chapter relied on the content of the previous chapter, so I got a free restart every few dozen pages or so.

But worse, some of the chapters show off what I would consider to be UGLY CODE and call it beautiful.  Flip to the chapter on Perforce.  It purports to be THE chapter on "beautiful code".  On page 530, the code is well aligned and looks neatly organized, but is chock full of Inappropriate Intimacy and Duplicate Code.   I mean, look at this snippit:
case MS_BASE: /* dumping the original */ if( selbits = selbitTab[ DL_BASE ][ diffDiff ] ) { readFile = bf; readFile->SeekLine( bf->start ); state = MS_LEG1; break; } case MS_LEG1: /* dumping leg1 */ if( selbits = selbitTab[ DL_LEG1 ][ diffDiff ] ) { readFile = lf1; readFile->SeekLine( lf1->start ); state = MS_LEG2; break; } case MS_LEG2: /* dumping leg2 */ if( selbits = selbitTab[ DL_LEG2 ][ diffDiff ] ) { readFile = lf2; readFile->SeekLine( lf2->start ); } state = MS_DIFFDIFF; break;
Really? Really. This is the best they could come up with for "Beautiful Code". It looks like some 1980's era hackers 4am jolt-ridden finger-barf.

Then, he starts advising people to avoid deeply nested code by using "Case Statements and Decision Tables".   Nothing about inheritance, nothing about refactoring code to pull out duplication.  Then he goes on to say that they don't refactor variable and method names on page 532 because they are afraid that it will introduce bugs.  So, they DON'T use a statically typed language that catches linking errors at compile time because the footnote say c++?  They don't have even a semblance of proper test coverage?  No wonder Perforce feels stuck in the 1990s; the software engineers are.

Avoid this book for anything other than a nice set of stories

No comments:

Post a Comment


Bookmark and Share