Hacker News Reader

Comment by diego_moita | Hacker News Reader

DON'T DO THIS!

When your code base grows you will end with dozens of places to change your code, all of them error prone. More things you need to keep control means more potential problems.

There are 2 alternatives to this. For languages that have macros (C#, C, C++) use the #ifdef construct, as others suggested here. For languages that don't have them (Java, Perl) use a less error prone alternative:

static boolean Test = false; // Only one place to change code

...

if(Test) doTest();

The difference in this last technique is that since you have just one place to change code there is less chance for errors and the change is easier to automate.

View on HN

You should not have commented out code checked in. Period. Whether you are doing if(0), #ifdef 0, #if 0 it doesn't matter. It makes the code harder to read and debug (suddenly whoever comes after you can't read the whole function / context on the screen at once).

If you DO want to do this either:

a) delete the code. It'll be in version control anyway right? You do do version control right?

b) put it under a NAMED feature flag. Test is not a suitable name. Put it in a separate file which is conditionally compiled into the build with #ifdef SHINY_FEATURE shiny_feature(); #endif

Almost all of the time I agree. But sometimes code is the clearest and most understandable way to describe an algorithm, and sometimes you want to describe an algorithm for a reason other than that you want it to execute. For example, you might want to say "This would be simpler but it fails when x is denormalized," or you might want to say "This is how you could call this method," which is common in Python and elisp docstrings — and, I think, wholly defensible in that case.

Most days I don't write comments. Maybe once every 3 months. When I feel like someone might need a comment I rewrite the code until I feel like they don't need a comment.

The only time I write comments is when:

1. Someone fucked up big and shipped an iPad app or something that relies on an error in the API or something. Can't force users to upgrade iPad apps, especially on older ones who are fearful of the app breaking or getting worse, so you end up with these massive comments fully explaining the problem, the hack that solves it, the conditions that need to be met in order for the hack to be solved (for example, "check google analytics to see if the number of people using version X of the iPad app or lower is less than 1%, if it is, replace the bellow hack with the following solution"), and possibly other hacks investigated and the reasons that they were declined. The reason you do this in the code rather than just in the commit message is that it protects the message from getting watered down as people change the code. I assume that someone is in a massive hurry when they are reading code and massive wtfs should be explained without further investigation.

2. When the performance of the obvious solution is terrible and the efficient solution is complex. Especially when math is involved (for example, the core match method of a distributed recommendation engine).

In cases where something is fundamentally broken but something "needs" to be changed now, I do this.

i.e:

// This is how it should be // foo(...) // But, since the code is horribly broken, we'll have to do: bar()

This is a neat trick, the best I've seen for commenting out huge blocks. But yeah, there is no reason for commented out code.

From "97 Things Every Programmer Should Know" http://programmer.97things.oreilly.com/wiki/index.php/Commen...

> Commented-out code is not executable code, so it has no useful effect for either reader or runtime. It also becomes stale very quickly. Version-related comments and commented-out code try to address questions of versioning and history. These questions have already been answered (far more effectively) by version control tools.

>a) delete the code. It'll be in version control anyway right? You do do version control right?

If only digging code up from a previous version was as easy as uncommenting it.

I'd appreciate if you'd have this discussion on Coderwall. That way I could 'defend' the post where it's due.

However though, please read https://news.ycombinator.com/item?id=5626456 and summary is:

It is a trick, not a silver bullet. It prevents code from being compiled to the binary, in a portable - yet hacky - way.

  #define DEBUG 1
  #if DEBUG
    test_code()
  #else
    real_code()
  #endif

Much more readable and the DEBUG macro can be defined in a common header, so all of the code can share it.

The comment trick isn't useless during development, but I'd see myself using a macro rather than trying to remember just where a slash goes.

Run that with vanilla JavaScript, Java, SQL or anything but C/C++ [this is the premise of the protip] and I'll bake you a cake! (One that's not a lie;) Nothing that's discussed here does the same thing, though I completely agree there is a lot of better techniques for various tasks, nothing is functionally the same as the trick. (Why can I not reply the the reply of this post? I'm new here.)

Well, first, there is nothing preventing you from running JS or Java through the C preprocessor (and we did run JavaScript through it at my last job).

But there are equivalents for any programming language, but others here have already touched on them.

I'm not sure what SQL has built in, but I'd just use the C preprocessor if I needed that. However, I'd much rather not muck with my queries like that, because that will cause nasty surprises in production.

And I don't eat carbs, so your off the hook for a cake.

> Well, first, there is nothing preventing you from running JS or Java through the C preprocessor (and we did run JavaScript through it at my last job).

This is an interesting idea! What was the problem you were solving that required this?

---

I think actually the C preprocessor would solve a lot of the problems that, say, SCSS solves too:

    // put these in a header file, 'cssutil.h'
    #define HASH #
    #define f(x) x
    #define COLOR(c) f(HASH)c

    #define PREFIXED(x, y) \
        -webkit- ## x : y;\
        -moz- ## x : y;\
        -o- ## x : y;\
        x : y
    #define SHADOWED PREFIXED(box-shadow, 10px)

    
    // then in your css
    #include "cssutil.h"

    #define THEME_BG COLOR(fff)

    div.main {
        background: THEME_BG;
        SHADOWED;
    }

There we go, pristine(ish) CSS, unsullied by magic constants and messy prefixes! (note that my preprocessor-fu is weak, I don't know how to do variadic prefixes)

Often times having a one-size-fits-all approach (especially across languages [!]) is not a good idea.

Different languages have different optimization techniques for removing code during compilation. Java, for example [1], can optimize out based on static boolean variables. I recommend learning more about each individual language and what it offers to get the best out of it.

[1] http://stackoverflow.com/questions/1344270/java-preprocessor

The road to hell is paved with convenience.

More sins have been committed in the cause of "Hey, this will save a few keystrokes" than . . . well, a lot of sins, okay?

I have no trouble doing this in the heat of bring-up or a debugging session, but I'd never check it in, and I hope that none of my cow-orkers ever will, either.

If you really want to save a few keystrokes, use a commenting plugin. Vim too :-)

The road to hell is paved with good intentions. :)

...and macros ;)

But he's right. It's a slippery slope to bad things when keystroke convenience takes precedence over clarity. We're human after all; it's easy to overlook something critical (especially when there's not another set of eyes going over it) and spend a lot more time and energy hunting for it later.

Agreed, use pre-compiler commands like this.

Alternatively if you don't need to check in your comments then your IDE should have a "comment selection" option (CTRL K, C chord in VS)

The disadvantage of using pre-compiler commands is that some IDEs (Xcode specifically, perhaps they all do this) don't color disabled code any differently, so at a glance you can't tell that the code block won't run.

Before a commit I'll sweep through and delete the intermediate code blocks that I've commented out. If I use pre-compiler flags to comment out the code, it's easy to miss it in the sweep. Because of this I never use the pre-compiler flags for something I don't want to get committed.

I like remapping 'comment' to ctrl-Q and 'uncomment' to shift-ctrl-Q. Makes for really quick editing of code while using the mouse.

Emacs's is M-;.

Oh, nice! I've been using C-c C-c but it doesn't work in every programming mode.

> When your code base grows you will end with dozens of places to change your code, all of them error prone.

Well, it's really only error prone if your editor doesn't have syntax highlighting. Seeing a whole swatch of code turn into the comment color pretty much tells you what's going on.

Still, I think personally I'd rather do an "#if 0". It's still only 1 character to toggle the code on or off...

I think the concern was more that the functions you are toggling will likely interact with other functions you are toggling. So if you toggle one on but forget to toggle the other, then you could end up with undesirable results.

I knew the original post left a bad taste in my mouth but wasn't sure of a better option. I really like this way better as it easily translates across any language.

My only issue using this with things like JS, won't all that code still be loaded into memory giving a misrepresentation of the memory profile of your app?

> My only issue using this with things like JS, won't all that code still be loaded into memory giving a misrepresentation of the memory profile of your app?

Yes it will. The alternative for that might be a modular design with very loose coupling and using Inversion of Control. Then, the only change you would need to make would be in the Inversion of Control configuration to load either the test code or the production code. You could place all the debug and trace code inside a single unit and use an empty stub in production. This would avoid sending the test code to the client on the production environment.

I am not an expert in JS, please correct me if this is wrong. I'd do something like this (for checking pre-conditions in method calls):

function Assert(condition, message ){} // this module is loaded/included in production only

function Assert(condition, message) { if(!condition) alert(message);} // this module is loaded/included in tests only

If you are sprinkling "if (test)..." throughout your code, you are doing it wrong.

Better would be to collect that code into one or two files at can be included when needed. I would imagine that's really only a concern when downloading code because the compiler should throw dead code ("if false...") away.

  if(Test) doTest();

Or something like

  if (USE_FEATURE_A) runFeatureA();

which allows for more specific, more fine-grain control.

Reply on news.ycombinator.com