No Comments on Funny Stuff
-
Jeffing again. Yay.
-
-
@Zecc good code can cleanly and clearly express what it is doing, but I never saw code that could explain the why without comments. And unless what it is doing is that trivial and that obvious (spoiler: it almost never is), the “why” is really useful.
But 11/10 for the image, cheered me up this morning.
-
@Arantor My main problem with comments is that the code will change with time but the comments won't, leaving you with comments that are obviously wrong or, worse, subtly misleading. Commit messages (and other context from the VCS) tend to be more reliable for figuring out the "why".
But I'll agree that there are times when a comment is the best option for explaining such things.
-
@ixvedeusi said in The Official Funny Stuff Thread™:
Commit messages (and other context from the VCS) tend to be more reliable for figuring out the "why".
"Fix"
"Small fix"
"Refactor"
"Further changes"
"Reverted for error"
"Implementation"
"Merge from feature/everything to develop"Not made up. Also some made by me.
-
@MrL It works well in projects like Linux and Git where the commit message is the main place to explain to the reviewer and the maintainer what the hell are you trying to do here and why they should merge it, so the commit messages naturally end up explaining it. In companies though …
… well, in most projects lately we maintained the rule that the commit message should mention the task the commit is made as part of, and that merging to master is only done via a merge request, which again references the task, so those two places should contain the relevant explanations instead.
… except half the tasks end up having a two word title and no description, so half a year later when you need to find out what the hell you were smoking when you last touched that code, you don't have a clue anyway.
-
@Bulb said in The Official Funny Stuff Thread™:
… well, in most projects lately we maintained the rule that the commit message should mention the task the commit is made as part of, and that merging to master is only done via a merge request, which again references the task, so those two places should contain the relevant explanations instead.
Wow, you have meaningful tasks. Cool.
… except half the tasks end up having a two word title and no description, so half a year later when you need to find out what the hell you were smoking when you last touched that code, you don't have a clue anyway.
Oh
-
@MrL said in The Official Funny Stuff Thread™:
"Fix"
"Small fix"
"Refactor"
"Further changes"
"Reverted for error"
"Implementation"
"Merge from feature/everything to develop"Also:
"Fuck"
"Why????"
"Damnit"
"Clean up"
-
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Luckily, here we have a policy to always include the code review ID in the commit message, and there I can usually find a bit more detail. And worst case, the VCS can tell me what other changes were made at the same time, which may give me a general idea what's going on.
-
@ixvedeusi said in The Official Funny Stuff Thread™:
Luckily, here we have a policy to always include the code review ID in the commit message, and there I can usually find a bit more detail. And worst case, the VCS can tell me what other changes were made at the same time, which may give me a general idea what's going on.
I go for descriptive text at the level of PR rather than in the commit. The PR comments probably start as a commit message, but often evolve to be rather more complete.
But then my actual comments tend to be accurate and up-to-date. I don't let them fester...
-
@Zecc said in The Official Funny Stuff Thread™:
@TimeBandit said in The Official Funny Stuff Thread™:
Good code needs no comments.
99% of code (in total, not per project) is too trivial to require explanation. The remaining 1% absolutely does need to have a comment why it's even there.
-
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
[edit]
This whole 'self documenting code' bullshit is extremely stupid. Like every fucking thing in IT - take a somewhat decent idea that may work in some situations and religiously push it everywhere with no exceptions.
: Hey, I'm reviewing your PR and I have just one thing - can you remove this comment?
: It explains why this seemingly idiotic thing is done in this flow.
: Well, yeah, cool, but you know, code should be self documenting.
: You can't make this ad-hoc-managerial-decision-idiocy self documenting.
: Um, yeah, that's bad, but the policy is self documenting code, so remove the comment.
Every. Fucking. Thing. Ruined by cargo culting idiots.
[/edit]
-
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
[Edit]
Though if necessary, yes I will go through all the commit messages for this file to find out what's going on.
@MrL said in The Official Funny Stuff Thread™:
Every. Fucking. Thing. Ruined by cargo culting idiots.
I do wholeheartedly agree with you on that one though. As I said above,
@ixvedeusi said in The Official Funny Stuff Thread™:
But I'll agree that there are times when a comment is the best option for explaining such things.
-
@ixvedeusi said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
There is no commit message you want to see. The file you are looking at was changed 120 times during last 2 years. Good luck piecing together what it does and why from that soup of messages.
Perhaps we work on very different projects.
-
I can and have written code sufficiently complex in my time that the “why this is correct” comments to code ratio was more than 1. I think I once peaked at 6:1 comments:code for “this is correct and here is why, leave it the fuck alone” level shenanigans.
Code should, ideally, be self-documenting, but most code that is, is also sufficiently obvious that it doesn’t need explaining.
-
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
There is no commit message you want to see. The file you are looking at was changed 120 times during last 2 years. Good luck piecing together what it does and why from that soup of messages.
Perhaps we work on very different projects.That's what
git blameand “pickaxe”,git log -Sandgit log -G, are for. They will give you the one of five commits that actually introduced the specific piece of code you are interested in. They work fast and give good results—provided your commit messages contain useful information, of course.
-
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
-
@boomzilla said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
Your Jira tickets are useful‽
-
@Bulb said in The Official Funny Stuff Thread™:
@boomzilla said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
Your Jira tickets are useful‽
They often end up documenting the dumb stuff the customer made us do, so yeah.
-
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
There is no commit message you want to see. The file you are looking at was changed 120 times during last 2 years. Good luck piecing together what it does and why from that soup of messages.
Perhaps we work on very different projects.My comments are pretty sparse and the commit messages are shit. Fite me.
Commits · Tsaukpaetra/concrescent
CONcrescent is the fandom convention registration system. - Commits · Tsaukpaetra/concrescent
-
-
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
Commit messages (and other context from the VCS) tend to be more reliable for figuring out the "why".
"Fix"
"Small fix"
"Refactor"
"Further changes"
"Reverted for error"
"Implementation"
"Merge from feature/everything to develop"Not made up. Also some made by me.
f9c0e58 Further various trimming a20f244 More fiddling 511e985 More cleanup and compactification e4a06cb General pruningIn my defence, I will be squashing this later.
-
@Bulb said in The Official Funny Stuff Thread™:
@boomzilla said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
Your Jira tickets are useful‽
When I make them, yes. When my cow-orkers make them...
-
@Bulb said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
There is no commit message you want to see. The file you are looking at was changed 120 times during last 2 years. Good luck piecing together what it does and why from that soup of messages.
Perhaps we work on very different projects.That's what
git blameand “pickaxe”,git log -Sandgit log -G, are for. They will give you the one of five commits that actually introduced the specific piece of code you are interested in. They work fast and give good results—provided your commit messages contain useful information, of course.
Let me repeat myselfThe file you are looking at was changed 120 times during last 2 years.
There is no specific commit that will tell you what this method does and why. If you want to learn this from commit messages (assuming they are 'good'), you have to go through tens of changes and piece in your head the whole history of this functionality evolution. Alternatively you could have 3 lines long comment specifying what it actually does currently. But no, that would be bad.
-
@MrL especially since I don’t usually care about the history of the world from the Big Bang to now, I care about why something is so now.
It should be the case that if you touch the code, you touch its accompanying comments, it’s not rocket surgery.
-
@Zecc said in The Official Funny Stuff Thread™:
Good code needs no comments.
I've seen many cases where the comments actually have negative value. They either restate what is already obvious or mislead by being incorrect/out of sync with the code itself.
I never saw code that could explain the why without comments.
The remaining 1% absolutely does need to have a comment why it's even there.
There's the key. Comments should explain why the code is needed not how the code works.
-
@MrL said in The Official Funny Stuff Thread™:
There is no specific commit that will tell you what this method does and why. If you want to learn this from commit messages (assuming they are 'good'), you have to go through tens of changes and piece in your head the whole history of this functionality evolution. Alternatively you could have 3 lines long comment specifying what it actually does currently. But no, that would be bad.
What is given by the code. Why is trickier. Or rather, most of the time the why is also simple, but occasionally it's very much not obvious. Why are particular constants chosen in a hash function? Why are we looking for the word "eroor" in the HTTP response body? Why are we releasing a lock at that point and reacquiring it a few lines down? Sometimes it helps to have text to explain rather than needing re-guess it from the test suite or commit log every damn time you look at the code, because not everything is obvious...
-
@dkf said in The Official Funny Stuff Thread™:
Why are we looking for the word "eroor" in the HTTP response body?
:zoidberg: Because your code is bad and you should feel bad.
-
@error Because their code is bad and should feel bad. (A coworker once messed up the wrapping of errors inside Rails. My code was just doing web service calls, but had to deal with several levels of idiocy in various failure cases.)
-
@MrL said in The Official Funny Stuff Thread™:
you have to go through tens of changes and piece in your head the whole history of this functionality evolution
IME I'll have to do that anyway because
@MrL said in The Official Funny Stuff Thread™:
Alternatively you could have 3 lines long comment specifying what
it actually does currentlysomeone sometime thought the code was doing at that point in time, even though it's obvious from the code that it currently doesn't and probably never did, and I'm still no wiser about the why they'd even ever wanted to do that .As I've said before, I'm not opposed to comments (as long as they don't just repeat what's obvious from the code) and they can be extremely helpful in some situations; but I have very little trust in them, and their presence or absence is, in itself, a very poor indicator of code quality.
-
We need more Funny stuff, so here is my all-time favourite code comment:
/* The Feature from the Black Lagoon! */If it helps, the code up to that point was scary, but code after that was much scarier...
-
@MrL said in The Official Funny Stuff Thread™:
@Bulb said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
your idea of finding out is to go through all commit messages for this file?
Have you ever heard of the magic of the "blame" command? It brings me right to the commit message I want to see.
There is no commit message you want to see. The file you are looking at was changed 120 times during last 2 years. Good luck piecing together what it does and why from that soup of messages.
Perhaps we work on very different projects.That's what
git blameand “pickaxe”,git log -Sandgit log -G, are for. They will give you the one of five commits that actually introduced the specific piece of code you are interested in. They work fast and give good results—provided your commit messages contain useful information, of course. Let me repeat myselfThe file you are looking at was changed 120 times during last 2 years.
There is no specific commit that will tell you what this method does and why. If you want to learn this from commit messages (assuming they are 'good'), you have to go through tens of changes and piece in your head the whole history of this functionality evolution.
I can accept that your mileage might vary, but my experience is that there always is just a few commits that are relevant for what I actually need to know and that they can be found in a couple minutes. Unless you actually need to know the whole history, which you generally don't.
Alternatively you could have 3 lines long comment specifying what it actually does currently. But no, that would be bad.
Such comments are great. Unfortunately they inevitably end up describing what the code did 50 commits ago, which is why such comments should generally not be trusted.
-
@PleegWat said in The Official Funny Stuff Thread™:
In my defence, I will be squashing this later.
That's not a defence, that's admission of crime.
-
@PleegWat said in The Official Funny Stuff Thread™:
squashing
One of these days I'll start using Real Branches and shit so that's a thing that can be done...
-
@Tsaukpaetra Branches make sense once you start to work on several things at once. They let you keep the various bits separate from each other.
-
@dkf said in The Official Funny Stuff Thread™:
once you start to work on several things at once
I have, at present, 16 things I'm working on simultaneously on that project. I'm not sure I can separate that into branches without further insanity.
-
@Tsaukpaetra said in The Official Funny Stuff Thread™:
@PleegWat said in The Official Funny Stuff Thread™:
squashing
One of these days I'll start using Real Branches and shit so that's a thing that can be done...
You know there's nothing stopping you from squashing all commits to master branch from the last month into one commit, right?
-
@Gustav said in The Official Funny Stuff Thread™:
@Tsaukpaetra said in The Official Funny Stuff Thread™:
@PleegWat said in The Official Funny Stuff Thread™:
squashing
One of these days I'll start using Real Branches and shit so that's a thing that can be done...
You know there's nothing stopping you from squashing all commits to master branch from the last month into one commit, right?
Ah, you haven't read much of the commit history it seems.
Filed under: rate of progress not guaranteed.
-
@Tsaukpaetra said in The Official Funny Stuff Thread™:
@Gustav said in The Official Funny Stuff Thread™:
@Tsaukpaetra said in The Official Funny Stuff Thread™:
@PleegWat said in The Official Funny Stuff Thread™:
squashing
One of these days I'll start using Real Branches and shit so that's a thing that can be done...
You know there's nothing stopping you from squashing all commits to master branch from the last month into one commit, right?
Ah, you haven't read much of the commit history it seems.
Filed under: rate of progress not guaranteed.
Sorry, I must've misinterpreted the "working" in "I have, at present, 16 things I'm working on simultaneously on that project".
-
@Gustav said in The Official Funny Stuff Thread™:
@Zecc said in The Official Funny Stuff Thread™:
@TimeBandit said in The Official Funny Stuff Thread™:
Good code needs no comments.
99% of code (in total, not per project) is too trivial to require explanation. The remaining 1% absolutely does need to have a comment why it's even there.
Despite what I said above, I like adding one line comments before longer blocks of code, just summarizing what they do. I find it helps navigating.
And yes, I do frequently add header comments in functions and methods telling what part of a larger process they are, if it's not obvious through their name.
-
@boomzilla said in No Comments on Funny Stuff:
@Bulb said in The Official Funny Stuff Thread™:
@boomzilla said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
Your Jira tickets are useful‽
They often end up documenting the dumb stuff the customer made us do, so yeah.
Yeah, that's usually my excuse.
Why was this done?
Go read the Jira. I'm not going through it again.To be honest, it probably should be the reason that your code changes most of the time. You get the occasional midwit who decides to refactor something and just ends up mudding the waters even more.
It's rare to go through the history to figure out how or why something is done. It's usually to blame to someone.
-
@DogsB said in No Comments on Funny Stuff:
@boomzilla said in No Comments on Funny Stuff:
@Bulb said in The Official Funny Stuff Thread™:
@boomzilla said in The Official Funny Stuff Thread™:
@MrL said in The Official Funny Stuff Thread™:
@ixvedeusi said in The Official Funny Stuff Thread™:
If you can't be bothered to write decent commit messages, your comments will be just as useless.
Not at all, I just consider commit messages as utterly useless. You look at a piece of code and you don't know why it is like it is - your idea of finding out is to go through all commit messages for this file? Really?
Ours include the Jira ticket they relate to, so yeah, it's often quite useful.
Your Jira tickets are useful‽
They often end up documenting the dumb stuff the customer made us do, so yeah.
Yeah, that's usually my excuse.
Why was this done?
Go read the Jira. I'm not going through it again.To be honest, it probably should be the reason that your code changes most of the time. You get the occasional midwit who decides to refactor something and just ends up mudding the waters even more.
It's rare to go through the history to figure out how or why something is done. It's usually to blame to someone.
Yeah, changes are tied to a ticket 99% of the time. But you don't always find a comment or other remark documenting why some particular detail was changed, unfortunately. Lots of stuff that makes sense at the time seems kind of ridiculous when you've lost the original context.
-
@boomzilla said in No Comments on Funny Stuff:
Lots of stuff that makes sense at the time seems kind of ridiculous when you've lost the original context.
That's actually a fair retort for a lot of the stuff around my neck of the woods. It solved the problem at the time and no one has a crystal ball that good. The stuff that people did try the crystal ball for usually caused more problems.
-
@Zecc said in No Comments on Funny Stuff:
@Gustav said in The Official Funny Stuff Thread™:
@Zecc said in The Official Funny Stuff Thread™:
@TimeBandit said in The Official Funny Stuff Thread™:
Good code needs no comments.
99% of code (in total, not per project) is too trivial to require explanation. The remaining 1% absolutely does need to have a comment why it's even there.
Despite what I said above, I like adding one line comments before longer blocks of code, just summarizing what they do. I find it helps navigating.
Even better would be splitting that longer block of code into multiple functions. Yes, I know that's not always possible, but whenever possible, that's the preferred way.
-
@Gustav Eh. C# has #region for single-use big blocks and most source code editors have code folding anyway.
"Whenever possible" is not to be preferred to "be sensible".
-
@Zecc I've never seen sensible code that uses #region. No matter how you fold up your code, it will always, always be less readable than multi-function version.
-
@Gustav said in No Comments on Funny Stuff:
Even better would be splitting that longer block of code into multiple functions. Yes, I know that's not always possible, but whenever possible, that's the preferred way.
If those functions have sensible names describing what they do, sure. But splitting just because 'that's the rule' and inevitably making up nonsensical names is just another one of those IT cargo cult things.
-
@MrL if you can't come up with names for the parts of your 250 line function, I don't think your one-line comments will do any better. Double so for #regions.
-
@Gustav said in No Comments on Funny Stuff:
No matter how you fold up your code, it will always, always be
less readable thanjust as unreadable as multi-function version.
The "long functions must always be split into parts" thing is bullshit. A function should always be conceptually coherent and meaningful on its own. It should be split up if it does several things or mixes different levels of abstraction. If there's no way to split the function into reasonably independent parts that can stand on their own, don't do it, it will only make you have to play connect-the-dots when you have to come back to work on it.
-
@ixvedeusi if your function only did one thing, it wouldn't be so long. The only exception is functions that are long because they're very repetitive, but they're so trivial they don't need any comments to start with.
