Hacker News new | ask | show | jobs
by h0bzii 2516 days ago
/* don't even try to load if not enabled */ if (!jit_enabled) return false;

I still think comments like these are super redundant and annoying.

4 comments

It tells you the intent. The actual code does exactly that, because the intent matches the code, but sometimes the two aren't aligned and therefore the "redundant" comment can help you determine if the bug is the code or the intent.

Understanding the "Why?" of code is often the most valuable thing about comments. The code remains forever, but the "Why?" is often lost to time.

Except that it does not. The 'why' would be explaining why the jit needs to be enabled to be able to load. As it stands it explains the 'what' and therefore is a bad comment.
A point well made. I conceded it could have been better written.

I still believe "redundant" comments can be of value though. This just may not be the best example.

> It tells you the intent.

A good unit test that tries to load with jit_enable=false and expects false in return will communicate intent even better.

Comments are usually less prone to logic bugs. When fixing issues, if the code behaves opposite to what the comment says, it can be a very helpful clue.
An argument can be made that this is what Test Driven Development is here to solve. State intent in the test, not the comment.

That said, I agree it can be a helpful contextual clue.

Heh, that's one of mine. The point isn't so much to restate the if itself, it's to explain that we're explicitly testing before loading the JIT provider - which happens immediately below.

Which in turn is because we a) do not want a hard dependency on any JIT provider, in particular we don't want a hard dependency on LLVM b) LLVM is a really slow to load dependency. Those bits are expanded more upon in the README in the same directory.

Edit: expand.

Agreed, sounds like trying to convince that imperative code is an easy reading. But OK, for some people it may be.