Replace Comment With Assertion

A refactoring in which you change a comment into an assertion. Described as IntroduceAssertion? in MartinFowler's excellent book, RefactoringImprovingTheDesignOfExistingCode. See http://www.refactoring.com/catalog/introduceAssertion.html for his example.

MartinFowler's use of IntroduceAssertion? is better than ReplaceCommentWithAssertion, because it applies even when there is no comment (but there should have been).

Example:

Before:

 // value must not be negative
 public double squareRootOf(int value) {
   ...square root algorithm...
 }
After:

 public double squareRootOf(int value) {
   Assert.isTrue(value >= 0);
   ...square root algorithm...
 }


Here, we could replace comment with type.

 public double squareRootOf(unsigned int value) {
   ...square root algorithm...
 }


That probably isn't what you want. It allows code like

  double root = squareRootOf(-1);

to compile and run quietly. The -1 is quietly cast to unsigned int, and then the square root is taken:

  double root = squareRootOf((unsigned int) -1); // square root of 0xFFFFFFFE


I'm going to argue that ReplaceCommentWithAssertion is not necessarily a universal GoodThing, and in fact is not my interpretation of MartinFowler's idea - there is a significant difference between the words "introduce" and "replace".

Sometimes, it can be difficult to assert a specific condition. The example given above seems to be a StrawMan because it is obvious that an assertion better serves the purpose in such a trivial example. For a counter-example, consider an algorithm that sorts over a list: should we ReplaceCommentWithAssertion here, and instead of writing: /* foo is sorted */ ? Or should we attempt to write an assert to check that each element of foo is less than the previous element, and that all the elements from the original list are preserved in the sorted list etc.?

My point is that comments and assertions are tools for achieving understanding of code. Assertions have the advantage they are enforced as code and form programmable safeguards, but then also have all the disadvantages of code: expression of abstraction can be verbose and non-trivial. In the above counter-example, our assertion may even end up being more complex than the sorting code itself. Comments allow for a higher level of abstraction in attempting to convey understanding to the reader (natural language, and all the pitfalls of it) but are not in any way "enforceable" as code or by the system.

Comments have their place and so do assertions: they are not mutually exclusive concepts. Comments are not intrinsically bad. Use the right tools for the job. Sometimes, yes, it does make sense to ReplaceCommentWithAssertion. Other times, it doesn't. I object to this page because it implies OneSizeFitsAll, which in this case is evidently not the case.

-- AlexJudge?

I think your example is problematic. If I understand correctly, you're using the example of /* foo is sorted */ in a sort function. Why would you ever do that? Presumably in a sort function, when the function returns, the collection is sorted. As for whether we should write an assertion that checks that it is in fact sorted, of course we should, but we shouldn't put it where the comment was: we should make it a unit test. In other words, the assertion that replaces a comment may not be in the same spot as the comment was.

Or are you suggesting /* foo is sorted */ as a comment in a function that acts on a sorted list? In that case, I'd still remove it, I think; I'd replace it with either a guard assertion or (better, if language permits) type-checking, such that List.sort returns a SortedList?. In general, if something is important enough to need a comment, it's probably important enough to find a way of making the comment executable.

--MarnenLaibowKoser, 17 Dec 2014


See AssertionsAsComments


CategoryRefactoring CategoryAssertions


EditText of this page (last edited December 17, 2014) or FindPage with title or text search