No Comments on Funny Stuff
-
@Gustav Counterpoint: I you can summarize what the function does in a line or two of comments, then it only does one thing.
-
@Gustav said in No Comments on Funny Stuff:
No matter how you fold up your code, it will always, always be less readable than multi-function version.
Passing a ton of local variables back and forth just for the sake of introducing a single-use function isn't pretty, let me tell you.
But you mentioned sensible code.Even in sensible code, having to jump back and forth to have a full picture of what's going on does not help readability.
But I refer again to the "be sensible" vague maxim. It goes both ways.
-
@ixvedeusi Countercounterpoint: if you can summarize what the function does in a line or two of comments, it doesn't need any comments explaining what parts of it do.
I wasn't saying splitting is always preferred to not splitting. I said splitting is almost (ALMOST, ALMOST) always preferred to writing the comments about what each part does.
-
@Gustav said in No Comments on Funny Stuff:
@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.
A firm believer I see. Cargo is bound to arrive any moment now.
-
@Zecc said in No Comments on Funny Stuff:
@Gustav said in No Comments on Funny Stuff:
No matter how you fold up your code, it will always, always be less readable than multi-function version.
Passing a ton of local variables back and forth just for the sake of introducing a single-use function isn't pretty, let me tell you.
Neither is following all the changes done to that ton of local variables throughout said long function. And when you hide those variables (or variable accesses) behind a #region, it only gets worse, not better. And if you really need to access all those variables everywhere, use a goddamn context object. Here, now you can pass however many variables you want with O(1) characters per inner function. It's not that hard.
Even in sensible code, having to jump back and forth to have a full picture of what's going on does not help readability.
And what do you think happens when your function doesn't physically fit on a screen? What happens when you tell your IDE to stop displaying part of the function? At least when you make separate functions, you can see with one glance how the individual parts are interconnected.
Better doesn't mean good. Splitting is always better than #regioning. Not always good.
-
@MrL said in No Comments on Funny Stuff:
@Gustav said in No Comments on Funny Stuff:
@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.
A firm believer I see.
Believer? I'm doing the exact opposite. I couldn't doubt your comment writing skills any harder.
-
@Zecc said in No Comments on Funny Stuff:
Jeffing again. Yay.
The thread title is a lie, it should be Comments on Unfunny Stuff.
-
@topspin said in No Comments on Funny Stuff:
The thread title is a lie
Do you see any comments on anything funny?
-
@Gustav funny "ha-ha" or...?
-
@boomzilla no comment.
-
@MrL said in No Comments on Funny Stuff:
: Um, yeah, that's bad, but the policy is self documenting code, so remove the comment.How about you fire
(and whoever approved that policy) into the sun?The idea of self-documenting code isn't bad. That just means that your code should, preferably, be readable enough that you don't need many comments explaining what it does. The mechanical part of what it does is documented by the code itself. As has already been said, commenting why it does what it does isn't excluded by that. Documenting interfaces, giving design rationales, etc., are all fine. It's just to avoid
foo++; // increment the foo counter.
Nobody sane would take that idea and come up with a policy of not allowing code to have comments.
-
@Gustav said in No Comments on Funny Stuff:
@topspin said in No Comments on Funny Stuff:
The thread title is a lie
Do you see any comments on anything funny?
Yes, the second post.
-
@topspin said in No Comments on Funny Stuff:
Nobody sane would take that idea and come up with a policy of not allowing code to have comments.
Have you met people? Or even worse - people in IT?
-
I have met people who fervently believe in “real programmers don’t need documentation”. I think every single one of them would be classified as an idiot.
The discussions here are borne out of experience and even those who argue for fewer/better comments are not arguing the cargo cult of “if you have to write docs you are doing it wrong” which is really the reductio ad absurdum version they’re aiming to land on.
-
@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.
With sufficient CRS, that context can be lost within a few minutes of making the edit.
-
@MrL said in No Comments on Funny Stuff:
@Gustav said in No Comments on Funny Stuff:
@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.
A firm believer I see. Cargo is bound to arrive any moment now.
± Delivery Distortion Field
-
@dkf said in No Comments on Funny Stuff:
@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"
@error_bot xkcd git commit
-
-
-
@HardwareGeek said in No Comments on Funny Stuff:
@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.
With sufficient CRS, that context can be lost within a few minutes of making the edit.
Hey, stop talking about me!
Also, that context is easily lost in an open office. "Hey, can you
talktake a look at this?" <poof/>
-
-
@HardwareGeek said in No Comments on Funny Stuff:
@dcon said in No Comments on Funny Stuff:
can you talk a look
No, I can't.
Yeah, I can proofread. Sometimes.
edit: And your answer is, of course, the correct one to give to the asker. Not that it didn't already induce sufficient CRS.
-
@izzion said in No Comments on Funny Stuff:
@error_bot said in No Comments on Funny Stuff:
is that git history order.
-
@DogsB said in No Comments on Funny Stuff:
It's rare to go through the history to figure out how or why something is done. It's usually to blame to someone.
That's why the function ended up being called
blame…@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.
The case when I go looking in the commit messages whether there is something relevant is when it turns out it doesn't just look ridiculous, but when it turns out to be wrong. That is, there is a bug, this piece of code is in the way of fixing it, it looks wrong, but … well, it might have been put in place to fix some other bug, or handle some special case, back then. So I go look in the commit messages, trying to find what it might have been so I don't reintroduce that bug or break that special case when fixing this bug.
Basically it's the Chesterton's fence scenario.
It is useful if the code has a comment about what it is working around, and I do write such comments, but the code might have changed since the comment was written, so I still check in the history whether that's the case (if I don't still remember).
-
@Bulb said in No Comments on Funny Stuff:
So I go look in the commit messages, trying to find what it might have been so I don't reintroduce that bug or break that special case when fixing this bug.
You're using commit messages as a substitute for unit tests.
-
@error said in No Comments on Funny Stuff:
@Bulb said in No Comments on Funny Stuff:
So I go look in the commit messages, trying to find what it might have been so I don't reintroduce that bug or break that special case when fixing this bug.
You're using commit messages as a substitute for unit tests.
If someone wrote a test for the case back when it happened and if the thing even can be unit-tested, it will still take a lot more time to fix it, run the test suite and find out what it broke. But the kind of problems I usually see can't really be unit-tested, only integration-tested, and that means that if I can even run it myself, the tests run all night. Or they are not automated and I need to ask the testing team. And for that I need to know what case it might have been, otherwise it will only be found during regression tests, which take two weeks. By which that time I won't even remember what I was fixing.
Compared to a few minutes digging up the records. And then making the right fix straight away instead of first making a wrong one and then redoing it better.
-
@error said in No Comments on Funny Stuff:
your code is bad and you should feel bad
I usually see you around bad code, do you feel no shame?
-
@Tsaukpaetra said in No Comments on Funny Stuff:
@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...
there are some people defending not using branches and using feature toggles instead. I think it sounds better, but nobody let me test it at any team yet. we have some issues with branches going for a year and getting unmergeable, and I kept doing the same work on multiple branches
-
@Gustav said in No Comments on Funny Stuff:
@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.
250 lines is perfectly cromulent if the function is simple
I have multi-thousand line functions that reuse global variables to solve memory in some 20 years old code to maintain
-
@sockpuppet7 said in No Comments on Funny Stuff:
@Gustav said in No Comments on Funny Stuff:
@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.
250 lines is perfectly cromulent if the function is simple
If the function is simple it doesn't need comments. If it needs comments it's not simple.
-
@sockpuppet7 said in No Comments on Funny Stuff:
@Tsaukpaetra said in No Comments on Funny Stuff:
@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...
there are some people defending not using branches and using feature toggles instead. I think it sounds better, but nobody let me test it at any team yet. we have some issues with branches going for a year and getting unmergeable, and I kept doing the same work on multiple branches
Both are useful tools in the arsenal. I can and have done both over the years, including in the same project. Having tools in the toolbox gives you more flexibility for given scenarios for sure.
-
@Gustav said in No Comments on Funny Stuff:
@sockpuppet7 said in No Comments on Funny Stuff:
@Gustav said in No Comments on Funny Stuff:
@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.
250 lines is perfectly cromulent if the function is simple
If the function is simple it doesn't need comments. If it needs comments it's not simple.
Hard disagree. Almost nothing I’ve ever seen is so trivial it doesn’t need comments of one fashion or another.
Even if it’s just to list the assumptions being made by a function.
-
@Arantor not sure if it's because you use PHP and other dynamically typed languages and I don't, but in my experience it's very rare that there are extra assumptions that need to be made beyond what's already in the function prototype. And anyway, I was specifically talking about comments that demarcate and describe individual parts of a function.
-
@Gustav said in No Comments on Funny Stuff:
@Arantor not sure if it's because you use PHP and other dynamically typed languages and I don't, but in my experience it's very rare that there are extra assumptions that need to be made beyond what's already in the function prototype. And anyway, I was specifically talking about comments that demarcate and describe individual parts of a function.
It’s not about typing, at least not in my experience.
Let me give you an example from the monster I am building at work. Part of the system is computationally intense, figuring out “given set of things A, and set of things B, solve for optimum A-B solution according to nasty algorithm”.
A and B are both represented by objects that are collections of objects in their own right. For reasons that are pretty nebulous to get into, certain assumptions around the makeup of these objects are assumed. B in particular is a collection of things where each of the things has an owner, a location and a type.
The real world asserts that it is possible for a single location owned by a single owner to have multiple quantities of a single type.
But the calculation engine expects, and does not vet, that the collection given to it is cromulent, it just assumes that it is, because it’s not the job of the engine to verify this is legit. But weird things will happen if it is not.
Meanwhile, the collection itself doesn’t verify this behaviour is correct when assembling the collection because to actually do so would start to invoke Schlemiel in quantities that are not ideal.
Now, I can manually collate the source data in such a way that guarantees I assemble the collection correctly in the first place which is what I do, but there are all sorts of warnings throughout the code comments about these assumptions (and yes, we do have unit tests to verify the assembly process, it’s just nasty enough to not do in real time with real world volumes of data)
But to your point, I’ve certainly written enough code over the years that is complex enough to warrant comments, where splitting it into functions wouldn’t prevent that complexity (and may well make it worse) and that there’s no good way to solve this with general guidelines.
-
@Arantor said in No Comments on Funny Stuff:
But to your point, I’ve certainly written enough code over the years that is complex enough to warrant comments, where splitting it into functions wouldn’t prevent that complexity (and may well make it worse)
Me too. But in specific case where the function is long and multi-part and you can put comments that split it into parts, splitting is good.
-
@sockpuppet7 said in No Comments on Funny Stuff:
@Tsaukpaetra said in No Comments on Funny Stuff:
@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...
there are some people defending not using branches and using feature toggles instead. I think it sounds better, but nobody let me test it at any team yet. we have some issues with branches going for a year and getting unmergeable, and I kept doing the same work on multiple branches
It depends on what you are doing. Branches are for things that are eventually supposed to converge—because the merge algorithm always tries to combine everything in the branches—while feature flags are for things that exist in parallel, e.g. modifications for different customers.
- For things that are supposed to converge, e.g. features under development, you want the ability to merge, so you use branches. You periodically merge the stable branch to the development ones to keep the amount of conflicts low. It also helps to learn what the 3-way merge algorithm does and resolve conflicts not by trying to understand the code around the conflict, but by simply applying the same merge logic with finer granularity than the automation can—it's both faster and you more often get it right.
- But for things that are supposed to live in parallel, the merge algorithm is going to get in your way. If you have two different customizations, merging one to the other will try to bring not just the changes in common code you want, but also the changes in the customization that should stay separate. But if you reject them, merging again in the same direction will not bring them again, but if you ever try to merge the other way, it will replace the customizations with the other ones, because merge is actually symmetric. So you'll only be able to propagate changes in one direction and you have to remember where to do all fixes first and how to propagate them. Feature flags are better here, because you just have one branch, just don't forget to test all the feature sets you need to keep working.
-
I think @sockpuppet7 meant in No Comments on Funny Stuff:
250 lines is perfectly cromulent if the function
isstretches the definition of simple
-
@Bulb said in No Comments on Funny Stuff:
It depends on what you are doing. Branches are for things that are eventually supposed to converge—because the merge algorithm always tries to combine everything in the branches—while feature flags are for things that exist in parallel, e.g. modifications for different customers.
If you have to manage two release/production series at once (e.g. 6.5.* and 7.0.*) then branches help plenty. You also need a lot of discipline to make that work; it's difficult. (It's downright diabolical without branching.) Feature enabling within that makes sense, but I mostly use it to handle platform-specific bits to let the rest of the code be platform-agnostic. Per-customer customisation is strictly downstream; it consumes the generic library/executable.
Development branches should either eventually merge or be closed and/or deleted.
-
@dkf said in No Comments on Funny Stuff:
@Bulb said in No Comments on Funny Stuff:
It depends on what you are doing. Branches are for things that are eventually supposed to converge—because the merge algorithm always tries to combine everything in the branches—while feature flags are for things that exist in parallel, e.g. modifications for different customers.
If you have to manage two release/production series at once (e.g. 6.5.* and 7.0.*) then branches help plenty.
That's a use-case for branches, and only branches, of course. And it in a sense is supposed to reconcile too—you should merge the changes from the maintenance release to the latest one (as long as the code to merge them into exists at all). Not the other way around, of course.
Feature enabling within that makes sense, but I mostly use it to handle platform-specific bits to let the rest of the code be platform-agnostic. Per-customer customisation is strictly downstream; it consumes the generic library/executable.
Depends on what you are doing. When the build produces a library/executable that you ship to the customer, it will have to already include appropriate feature selection for that customer.
-
@Bulb said in No Comments on Funny Stuff:
When the build produces a library/executable that you ship to the customer, it will have to already include appropriate feature selection for that customer.
Sure, but I'd go with having that be done from a separate repo that holds just the customizations. Like that, they can't be accidentally put where they don't belong. Mind you, I don't come from a world-model that thinks that monorepos are a good plan to start out with...
-
@dkf said in No Comments on Funny Stuff:
Development branches should either eventually merge or be closed and/or deleted.
Sigh. We currently have over 7400 "active" branches. Mainly because people forget to check the "delete on merge" box on the PR. Makes using the GUI I use to set the upstream branch useless because a combobox with that many entries <choke/>...
-
@dkf said in No Comments on Funny Stuff:
@Bulb said in No Comments on Funny Stuff:
When the build produces a library/executable that you ship to the customer, it will have to already include appropriate feature selection for that customer.
Sure, but I'd go with having that be done from a separate repo that holds just the customizations.
You are assuming that the customizations can be put in a separate layer without turning the whole logic inside out and making it an unreadable mess. In projects I worked on you couldn't do that. The customer-specific parts were all over the lower levels, by necessity, and trying to inject them with policy objects or something would make it completely unreadable.
Like that, they can't be accidentally put where they don't belong.
It's nice to have clear separation for sure, if you can, but directories are just fine for that. Especially when the core can't be built for no customizations at all, which the ones I worked with definitely couldn't.
In the second one, each customer had a different build system and different API that the software was connecting to and there was no standard for that API, so there was no sensible “default” to test against before applying any customizations. They were not “platforms”, it was still armv7-none-linux-gnu, but they were platforms in the kind of things the software had to adapt to.
Mind you, I don't come from a world-model that thinks that monorepos are a good plan to start out with...
Whether monorepos are a good plan to start out with depends on how your work is organized.
With single repository, things are guaranteed to only be used in combinations that are together in a single revision. That lets you change internal interfaces without any regard for compatibility, which makes it easier. So if your customizations need hooks deep into internal logic, even if they are written as separate policy classes, you want to keep them in the same repository.
But when it gets too big, and has too many people working on it, they start stepping on each others toes a lot. Especially if they are working on otherwise unrelated things and now have to talk to each other much more.
So you split the repository, but than it means you have to start versioning the interfaces between the components in different repositories and have some kind of release process, even if relatively simple, when you change those interfaces. Otherwise you'll be seeing a lot of test failures just because wrong versions got linked together.
Which is why I think the optimal approach is to have the structure of repositories match the teams. So when you have a core team and then customization team(s) doing the builds for different customers, you want separate repositories, while when you have one team building different feature sets, it should still be just one repository.
-
@dcon said in No Comments on Funny Stuff:
Mainly because people forget to check the "delete on merge" box on the PR.
There should be a “delete all merged branches” button somewhere to clean that up. It definitely is there in GitLab.
-
@Bulb said in No Comments on Funny Stuff:
There should be a “delete all merged branches” button somewhere to clean that up.
There isn't in Github (unless you set up a workflow to do it) but you can go to the list of branches and see which are merged (or otherwise associated with a closed PR) easily enough. As long as you haven't let things build up too much, it's non-onerous.
-
@dkf said in No Comments on Funny Stuff:
There isn't in Github
(yeah, github has actually always been kinda lame).
1296: Git Commit - explain xkcd
