Hacker News Reader

Comment by RyanZAG | Hacker News Reader

You really shouldn't have to be relying on history for all of that context. It should definitely have been a function simply called 'triggerLayout()'. Then the exact and best method for triggering layout could be put in that function and used throughout the project where necessary, and easily updated if a better method of triggering layout comes along.

Code like this is extremely brittle with or without that git history, don't rely on it like a crutch as it makes the code obscure, and updating a line somewhere may leave other similar lines updated or not. If you wanted to update the triggerLayout function, you would have to go through the git commit log for every .clientLeft line to see if that one was or wasn't used for triggering layout...

View on HN

"Code happens". In reality, there's code that should have been commented all the time and even in the best codebases. I don't think there's a coder in the world who hasn't had a time when he's looked back on a piece of code and just thought "what?". If you get in the habit of keeping a well documented git history, it's an invaluable resource. I find on top of these benefits, having to explain what changes I made also helps me make smaller and easier to understand commits, which also helps with summarizing releases, finding commits where bugs were fixed or introduced, and so much more. A well documented git history is the closest thing to a time machine you can get.

Should these things happen? No. Do they happen? All the freaking time.

What? No, it's got nothing to do with commenting. It's to do with basic code design: put your stuff in functions. A line like that should have been in a function to start with, and it should have been caught during a code review or as common sense by the committer before he even committed.

Agreed. Maybe it's in some insane tight loop where you don't want a function call - in which case you comment it. My rule is that if there's something I have to put in the code that's out of the ordinary, I comment to explain.

The point of the article has nothing to do with the code snippet. My point has nothing to do with the code snippet. It's just about code in general, and it's never as easy to understand as you want it to be.

basic code design: put your stuff in function

Aye, and with intention revealing names!

http://c2.com/cgi/wiki?IntentionRevealingNames

Also: http://c2.com/cgi/wiki?LawOfDemeter

Thanks for your words of defense, etler. A lot of the criticism of my original post was valid, though, so I addressed it by rephrasing and stressing out that code quality still matters: https://github.com/mislav/mislav.github.com/commit/e07907966...

You were right, however, that the post was never about code quality or code comments. It was only about "history's great, yo; here's what you can do with it".

Honestly they should just have a comment:

// This triggers layout. Needed for some browsers.

But agreed that you shouldn't require relying on history for this context. It's bad news. Yes code happens, but it should be obvious what it does (via clear code) and why (via comments as needed).

I was recently watching a talk given by Rich Hickey where he talks about serialization formats and how they can have all their metadata in-band or have to depend on out of band information. The latter situation is where bugs can be introduced.

The example in the original post relies heavily on out of band documentation. Your suggestion of an intention revealing function name puts the same info in-band.

One should always ask themselves: Does my code depend on out of band information? If so, can we correct this?

That may be true for the given example, but it doesn't work that way for all the nuances of code that come up. For one thing because English wording has ambiguities, and for another because there are so many concerns with some cross-cutting each other that you can only break things down into atomic functions to a certain degree.

I think the overall point of the article rings very true: great VCS habits pay dividends over time regardless of how well-crafted the code itself is.

Way to miss the point completely.

But he is actually right: when dealing with obscure implementation details, the best thing to do is to encapsulate it properly, the minimum is to comment it clearly, and ideally detailed explanations should be written in the commit message.

Having a detailed code history isn't in any way an excuse to leave obscure code as is. If you have time to document it, you have time to make the code right.

Of course he is right. But he still completely missed the point of the article, which was not to defend writing or keeping code that is not self explaning. I mean, it's the goddamn premise of the article that you happen to stumple upon a piece of code that's less than perfect! The example in the article describes a situation in which you find a piece of code someone else wrote and you are unsure of what it does or why it's there.To even begin contemplating improving the code, you first have to understand it. The article is about how you can do that with Git logs. So that once you've done that you may proceed to improve it. And yes, extracting it into a well-named method is probably a good way to do that. But that, as I hope we've established, is besides the point.

The premise of the article is that you happen to stumble upon a messy piece of code and that the explanations related to it might be located into the source history. Yet the only reason why these explanations would actually be there is that someone would have enforced doing that as a rule. And the point of the counter-argument is that before enforcing this rule this same person should be enforcing a rule saying that writing messy code like that is prohibited.

In short, maintaining a clean code base has always higher priority than maintaining a clean log.

That's like saying that a "counter-argument" to wearing a seatbelt is that drivers shouldn't be getting into accidents in the first place. Yes, it would be nice if all code was well-commented and well-factored with well-named functions, classes and variables, and we should all strive to make code like that. And if that never failed, then perhaps we wouldn't need any other best practices or rules to help us along.[1] But we all know the real world is not like that. We need all the help we can get. Having good practices for writing good code is not, I repeat not an argument against having good practices for commit comments. You make it out to be an either/or proposition. It's not.

jesus christ

http://lsolum.typepad.com/legal_theory_lexicon/2003/09/legal...

this article is about recommendations of what to do ex post once a comment-less line of code has been committed in the past that you need to understand. arguments about ex ante things such as how it got there in the first place and how to prevent it from happening is completely orthogonal to the point of the article.

Strong disagree. The article is about recommending and making ex post use of a policy for good commit messages. The point argued is in favor of a future rule for good commit messages. Therefore it is very reasonable to suggest a superior rule for the future.

As rymohr indicated, the author of the code given as example is actually the author of the article himself:

https://github.com/madrobby/zepto/commit/3d92f20966aa02dee82...

Which means the OP genuinely thinks that commits like this one are good practice, and the purpose of the article is to show how to deal with it.

Yet as it was argued above, commits like that should never happen in the first place.

And it didn't. This was the commit that actually happened: https://github.com/madrobby/zepto/commit/2ed0123eaddc023a857...

Notice the code comment. I took it out for the example in the blog post to illustrate how we would deal if there was never a code comment in the first place.

From what I've seen in the article, the point was to present ways of getting documentation out of commit messages, not implying that they should be used for documenting (which I would disagree with, too).

I think he got the point. Just that the article is predicated on something incredibly stupid: someone writing a line of code that, to probably most people other than the author, is useless and incomprehensible. I think it's great that the author wrote a detailed explanation in the git commit message, but it's absolutely not enough. That piece of code needs at least a one-line comment there, full stop.

I think the article as a whole is great in that it taught me some things you can do with git that I didn't know before (well, stuff I figured was possible, but didn't know the magical incantations). But it's hard to start reading it and really appreciate all that when the initial premise for explaining all that stuff was... well, someone having a severely bad day when it comes to writing clear code.

> That piece of code needs at least a one-line comment there, full stop.

From my experiences in the real world and from reading comments here, I think there are a lot of people that disagree with that assertion, and it kind of makes me wish I worked in a field where I don't feel like I'm going insane on such a regular basis.

It seems that this was the only instance of its use in the whole codebase. I certainly agree that a comment should have been placed there, and of course creating a function would be the most idealistic solution. But I think that, without knowing the specific context, it is hard to say whether creating a function would have been an improvement or detriment to the code -- especially considering it is just a single line of code.

Reply on news.ycombinator.com