Notted Up

There’s an old saying, that if your code is so unclear it needs comments to explain it, you should probably rewrite it. Dan found this code in a production system, which invents a bizarre inversion of that principle:

static BOOLEAN UpdateFileStoreTemplates ()
{
  BOOLEAN NotResult = FALSE;

  NotResult |= !UpdateFileStoreTemplate (DC_EMAIL_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (DC_TABLE_HEADER_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (DC_TABLE_ROW_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (DC_TABLE_FOOTER_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (WS_EMAIL_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (WS_TABLE_HEADER_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (WS_TABLE_ROW_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure
  NotResult |= !UpdateFileStoreTemplate (WS_TABLE_FOOTER_TEMPLATE); // Not-ing a fail makes it true, so if Not result is True we've had a failure

  return !NotResult;
}

Here, the code is clear enough that I don’t need comments, but the comments are so unclear I’m glad the code is there to explain them.

Not-ing a fail certainly does not make a win.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!