Hacker News Reader

Hacker News

Comment by chollida1 | Hacker News Reader

Full thread

chollida1

I opened one package at random

http://opensource.apple.com/source/IOFWDVComponents/IOFWDVCo...

I see this:

#include <Carbon/Carbon.h>

//#include <Resources.h>

//#include <OSUtils.h>

//#include <Components.h>

#include "DeviceControlPriv.h"

#include "IsochronousDataHandler.h"

#include "DVVers.h"

#include <stdio.h>

#include <stdlib.h>

//#include <syslog.h>

// Debug messages

It's good to know that I'm not the only one who has commented out code in their shipping code:)

View on HN

Trust me, you're among friends.

You have no idea. I know git is supposed to mean I can delete code without fear of losing it.

Still...

When I comment out code, I add a timestamp of when I disabled it. Then, when I come across commented code with a timestamp more than 6 months old, I go ahead and delete it. Best of both worlds!

Oh lord.

Best demonstration of Poe's Law right here.

Doesn't "git blame" take care of your timestamps for you?

no. vc will only take care if you have the history and know what you are looking. if you publish a final version only like this all that is lost. if you run a interpreted language you can't find the place of the old code while debugging at runtime.

there are many reasons to leave code comments besides neglect. though in this case it's neglect :)

Hmm what a nice extension idea for emacs comment-dwim.

You still can put one on top of that, like this:

    #if 20141031     // Date as a valid number != 0
      // Active code
    #else
      // Inactive code
    #endif

Helps documenting experimental alternatives, even without SCM.

This is really terrible practice. You should only do this if you really have no SCM available.

I'm talking about experimental alternatives. This is not for production code. And it's still better than just out-commenting code to deactivate it.

This is a similar to an idea that David Schnepper talked about at Velocity Ignite this year.

It's only a five minute presentation but it blew me away and I'm keen to give it a try for real at some point. Video here:

https://www.youtube.com/watch?v=29UXzfQWOhQ

I like this alot, would anyone be keen to collaborate on a simple ruby gem to implement this 'tombstone' method?

great idea, I would definitely throw it into all of my ongoing projects.

I put this together today, would value any feedback and/or contributions:

https://github.com/lewispb/tombstone

Wow, simple and short, but has already given me a ton of ideas. Saved and shared with our IS department too.

Funny how this is kinda Halloween-ish.

Great share, thank you!

This is thought provoking; I shared my reactions here: http://djwonk.tumblr.com/post/101423057084/tombstones-as-a-w...

I wonder how they tombstone CSS.

I like that idea. I remove commented code when it's "old enough" but know how long it's actually been could be really helpful.

I would be awesome if there were a plugin (for your favorite text editor) for that. It should be easy to add automatically and remove when you uncomment it.

Perhaps not a bad idea. One advantage to David Schnepper's suggestion (in the video above) is that the tombstone is executable code, which can lead to metrics and even automated code removal. Buyer beware!

Glad to hear this isn't an odd concept. When I see others have commented out code in our codebase, I do a quick git blame (Sublime Git ftw!) and if it's from the previous year I'll just delete it.

We aggressively delete code. Hg/git will have it if you ever need it.

We never have.

This. Though I do often wonder how often I re-type the same thing, because I can't simply uncomment, simply because it's easier to type it out than it is to go hunting through the history. Maybe I should try to make a Git history "redone undos" script or something.

When I actually think about it I realize it's probably very nearly never. But it's hard to shake the suspicion :)

It's just more convenient to keep the code in, otherwise you'll have to look it up again which takes more time... (and effort, we're developers after all)

This was the one thing that my CS professors drilled into my head most: you have source control, why would you comment?

You only make your source control more valuable by not commenting out code; since you can then search for its presence more easily. Good advice, all around.

I think it's genetic :) Some people are "code pack rats". They're afraid to delete anything.

When you put a lot of time and effort into adding code in all the right places to provide for functional, helpful debugging, tearing it all out afterwards is just going to make you add it back in again when debugging something else down the road.

Even if you tear it out, it's way more work to try to pull it all back out, line by line, and re-implement it than to comment it out and leave the actual debugging statements in #ifdef blocks that the pre-processor will pull out anyway.

This is bad practice, at best it shows a lack of confidence in what you've written. At worse it shows you don't understand what's going on.

Version control shows the evolution of design, if you are leaving in dead code you need to take a step back and think about the real solution.

Does anybody know what Apple uses internally as their version control setup? I did some Google searching that seems to suggest they use Git, but I'm curious if anyone knows more.

When I was there it was a healthy mix of CVS, SVN and git.

Engineering wanted to move to git, so I would assume they are on the path to 100% git, annnnd.... probably already are at this point.

There is still some SVN floating about but its primarily git

There's a great deal in Perforce too; the P4 gui tools are excellent, by the way, at illustrating changes in source over commits, and are incessantly in use.

I've heard this so much. Usually from people who reinvent the wheel five times over.

I've never seen anyone dig through VCS history to gain a better understanding of code. And since presumably things change because the understanding evolves, I'm not even sure what the thought process is that would lead someone to do that.

You comment out code because your understanding might not be perfect, you might want the option to easily roll-back, or even have a reminder that a particular change was made. You'll remove it in the next revision or two.

So yeah. I've heard this before. It's complete and total nonsense IME.

> I've never seen anyone dig through VCS history to gain a better understanding of code.

git bisect, git log, and git blame are all indispensable tools. To counter your anecdote, I use them all the time and see others do so.

There's really no reason to leave dead imports in a file.

> I've never seen anyone dig through VCS history to gain a better understanding of code.

I'm actually going through the ancient CVS history of Mozilla Firefox in order to learn about a relatively poorly understood piece of code that's still in there to this day:

http://mikeconley.ca/blog/2014/07/07/docshell-in-a-nutshell-...

and

http://mikeconley.ca/blog/2014/08/10/docshell-in-a-nutshell-...

You never wonder why particular lines of code happen to do exactly the things they do, in the exact order they do them, when your intuition would tell you that it would be a third as many lines if only the code called this or that utility function, or that it obviously doesn't handle the particular corner case of a new bug report? You must work on unusually well commented code bases then; I wouldn't get anything done without git log -p... at a minimum you will quickly find out which generation of maintenance programmer added the odd code in question and what (if any) bug it was intended to fix, so you don't break that use-case. Well, unless you're out of luck and the first generation of them lost the first 5 years of SCM history, that is...

No. If the intent behind the source code you're actually looking at is not apparent, then realistically, looking at it's history is probably equally useless.

If there aren't tests or specifications to cover the code, I generally assume any bugs are because of a lack of understanding and proceed from there.

Scrolling backwards through history and looking at progressively more naive or prototypical implementations isn't going to tell me anything except the code was likely underspecified IME.

Comments are infrequent, but when I write them, they're usually to try to clarify particularly complex implementation detail. Not to communicate requirements. Comments aren't the ideal place for that IMO.

> I've never seen anyone dig through VCS history to gain a better understanding of code.

One of my favourite interview questions:

    A bug is found where given a specific series of user
    interactions an error is raised. You search for this error
    in code and you find that the error is raised in a very
    old module which was written in a different style from
    the main codebase by a developer who has long since left
    the company. From a quick look around you see that this
    code is very complex, and probably quite important. It's
    obvious that it will take quite a bit of time to 
    understand what's going on simply by just reading the
    code in the module where this error is raised.

    Your job is to fix the bug. How do you approach this
    problem?

At my company digging through VCS (and our ticketing system) in order to gain a better understanding of code is a very common.

Edit:

I think if you're leaving commented out code for history's sake, you're Doing It Wrong. I disagree with the GP's idealism as much as you do, however. My beef with it is that if you're checking in commented code, you're obviously not reviewing your own diffs. It means you're likely to commit minor changes which were made for debugging purposes.

    // comment this, just to get this stupid module out of my way.
    //ensureNobodyDies();

Ticketing system sure. If you have the full history you'd get the original requirements hopefully, and revisions or issues along the way.

VCS. Maybe it's just a Web vs Other development thing. The history of an Action in Rails is going to be someone using mass-assignment unsafely. Or a dozen stylistic tweaks. Or implementing a pattern. Or removing it.

It's about the worst possible way I can imagine to attempt to understand requirements.

What I do find helpful is "git blame", and just talking to the person who implemented it. And if they aren't available, then hopefully someone who was there at the time, and involved with the feature, is. At least I have a time-frame.

As a last resort, if no one has any clue what's going on? Scrolling through VCS diffs would be an act of desperation perhaps. But I'd much rather look for tests. Automated or a Manual Test Plan. A Specification. Or just use common sense.

At the end of the day it's most likely that I'm tasked with this issue because the original implementation was a scheduling compromise, or the original developer just couldn't think of how to make it simple, so they made it complex.

Actual complexity is rarely justified by requirements I find. Unless I'm the one writing it of course. ;-) Then it's absolutely the only sane way to implement it and you just need to try harder to understand. Duh.

> Actual complexity is rarely justified by requirements I find. Unless I'm the one writing it of course. ;-)

I appreciate your humility, and I largely agree with your approach. You'd probably do quite well in my interview.

With respect to actual complexity rarely being justified, I'm presently a team lead for a system which involves fairly precise modeling of some very piecewise real world processes under very tight performance constraints. There are so many things we just can't generalize, either because there's no clean abstraction, or because abstraction comes with too much inefficiency. We try to keep our processes from smelling too much like old stale spaghetti, but unfortunately some things are just simply complex, and sometimes in the face of such complexity the most elegant and/or performant solution can make purists cringe.

I'm the converse, I break other rules, but I really don't commit commented code (well I have some demo code commented somewhere, but it's runnable code).

I thought Carbon was already deprecated in favor of Cocoa - does that header mean it's still being used in the Yosemite?

Carbon applications are deprecated, but there are C-based APIs still in use that fall under that umbrella term, such as Keychain Services.

Understood. Thank you.

Commented code, yuk.

Exterminate it with a flamethrower

Reply on news.ycombinator.com