Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

That article doesn't make a compelling case.

It suggests taking this code:

  // square root of n with Newton-Raphson approximation
  r = n / 2;
  while ( abs( r - (n/r) ) > t ) {
      r = 0.5 * ( r + (n/r) );
  }

  System.out.println( "r = " + r );
And refactoring it to this function:

  private double SquareRootApproximation(n) {
      r = n / 2;
      while ( abs( r - (n/r) ) > t ) {
          r = 0.5 * ( r + (n/r) );
      }
      return r;
  }

  System.out.println( "r = " + SquareRootApproximation(r) );
I'm all for this refactoring, but something was lost in the process. What kind of square root approximation is being used? Does the algorithm have a name? What would I search for if I wanted to read more about it? That information was in the original comment.


There's an infinite amount of detail that's impossible to capture in a comment and which invariably changes over time and doesn't hold in the future.

For my team, the solution has been writing longer commit messages detailing not only what has changed, but also the why and other considerations, potential pitfalls and so forth.

So in this case, a good commit message might read like:

``` Created square root approximation function

This is needed for rendering new polygons in renderer Foo in an efficient way as those don't need high degree of accuracy.

The algorithm used was Newton-Raphson approximation, accuracy was chosen by initial testing:

[[Test code here showing why a thing was chosen]]

Potential pitfalls here include foo and bar. X and Y were also considered, but left out due to unclear benefit over the simpler algorithm. ```

With an editor with good `git blame` support (or using github to dig through the layers) this gives me a lot of confidence about reading code as I can go back in time and read what the author was thinking about originally. This way I can evaluate properly if conditions have changed, rather than worry about the next Chthulu comment that does not apply.


Now you have to look in two places for the information- the code and the commit messages.


How so? The code still documents what is happening, the commits however lay out the whys.

The point is that these two are separate questions, and that trying to use comments as a crutch to join the two religiously is a headache. It's impossible to keep everything in sync and I don't want to read needless or worse misleading information.

What's worse, in comments we often omit the important details such as why was the change made, what other choices were considered, how was the thing benchmarked, etcetc.

That said, comments still have a place. Just not everywhere for everything and especially not for documenting history.


I disagree. I think the "whys" belong in the comments- in fact, that's the most important part of the comment if the code is cleanly written. I don't want to be happily coding along, get to a glob and have to go to the repo pane, hunt for the commit that explains this particular thing, then read a commit message. Put it in a comment in the code. Pretty please.


That isn't actually important unless you have multiple algorithms, which is when you create an ISquareRootApproximator interface, and have NewtonRaphsonAlgorithm and any other classes implement it.

Then you can have them duke it out running identical unit tests in the profiler.

That creates a clean separation between the people who just need to know what a function does and those that need to know how that function works. People who just need an approximated square root fast can understand perfectly well what ISquareRootApproximator.ApproximateSquareRoot( r ) does, and don't necessarily care whether your "get me a square root approximator" function returns a NewtonRaphsonAlgorithm, or a CORDICAlgorithm, or a BitshiftsMagicNumbersAndLookupTablesAlgorithm, or something else, so long as it approximates a square root.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: