5 ms·

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

by RyanZAG 13y ago

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...

etler 13y ago
"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.
- RyanZAG 13y ago
  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.
  - etler 13y ago
    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.
  - aidos 13y ago
    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.
  - stcredzero 13y ago
    basic code design: put your stuff in function Aye, and with intention revealing names! http://c2.com/cgi/wiki?IntentionRevealingNames http://c2.com/cgi/wiki?IntentionRevealingNames Also: http://c2.com/cgi/wiki?LawOfDemeter http://c2.com/cgi/wiki?LawOfDemeter
- mislav 13y ago
  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/e07907966bc9e3e46d87346debca8d3d6d1228f1#rdiff-803d102386e332a6f64ffd2fddafdca2 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".
adamlett 13y ago
Way to miss the point completely.
- jpatte 13y ago
  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.
  - userpasswd 13y ago
    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).
  - adamlett 13y ago
    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.
    - jpatte 13y ago
      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.
dasil003 13y ago
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.
shawnz 13y ago
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.
einhverfr 13y ago
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).
stcredzero 13y ago
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?

by RyanZAG 13y ago

etler 13y ago

"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.

RyanZAG 13y ago
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.
- etler 13y ago
  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.
- aidos 13y ago
  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.
- stcredzero 13y ago
  basic code design: put your stuff in function Aye, and with intention revealing names! http://c2.com/cgi/wiki?IntentionRevealingNames http://c2.com/cgi/wiki?IntentionRevealingNames Also: http://c2.com/cgi/wiki?LawOfDemeter http://c2.com/cgi/wiki?LawOfDemeter
mislav 13y ago
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/e07907966bc9e3e46d87346debca8d3d6d1228f1#rdiff-803d102386e332a6f64ffd2fddafdca2 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".

adamlett 13y ago

Way to miss the point completely.

jpatte 13y ago
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.
- userpasswd 13y ago
  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).
- adamlett 13y ago
  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.
  - jpatte 13y ago
    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.

dasil003 13y ago

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.

shawnz 13y ago

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.

einhverfr 13y ago

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).

stcredzero 13y ago

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?