I wonder if this arises there because people get moved and would otherwise receive lots of clarification requests regarding the implementation from a new maintainer. It is easier than maintaining a separate wiki page that might lose track of changes.
Please do not attempt to simplify this code
231–240 of 647 posts
Re: Please do not attempt to simplify this code
#232I love this! It's the "jazz music" of software development. Something which breaks all the "rules" but does so purposefully and explicitly so that it can become better than the "rules" allow. A naive look at this and my head is screaming that this file is way too big, has way too many branches and nested if statements, has a lot of "pointless comments" that just describe what the line or few lines around it is doing,…
> I love this! It's the "jazz music" of software development. Something which breaks all the "rules" but does so purposefully and explicitly so that it can become better than the "rules" allow.
I kind of see it as the opposite: “space shuttle style” is code that adheres to heavyweight rules that most software development has abandoned in favor of a more improvisational style.
But in either case it illustrates that code style rules are desirable, or not, based on the context; you need to understand what purpose rules serve and what trade-offs they involve to understand which rules to use; there's no one-size-fits-all solution.
Re: Please do not attempt to simplify this code
#233Earlier quoted context omitted.
I agree. My day job is working on code that isn't this level of critical, but also has the characteristic of being low level, both closer to the metal than typical backend code and also called by so much frontend and backend code that if there was such a thing as "even backend-ier code" this would be a good example. If you miss a nuance, a horde of angry developers will show up at your desk the moment the build deplo…
Note to others: 3rd order is not necessarily 10^10^10. It can easily be 10^100^1000.
[deleted]
Re: Please do not attempt to simplify this code
#234Space shuttle coding is like preparing code for your future self to quickly make sense of.
A key part of the software lifecycle is being able to easily onboard newcomers to be productive, or coming back to a part of a code base months, or even years later.
Re: Please do not attempt to simplify this code
#235 if claim.Spec.VolumeName == "" {
// ...
} else /* pvc.Spec.VolumeName != nil */ {
Re: Please do not attempt to simplify this code
#236One simplification that might be tempting is to replace the "if !condition" with "if condition".
For example, line 463 shows:
if !found {
// handle missing
} else {
// handle found
}
if found {
// handle found
} else {
// handle not found
}
if missing {
// handle missing
} else {
// handle not missing
}
Re: Please do not attempt to simplify this code
#237Earlier quoted context omitted.
> It's certainly not that McDonald's makes better (or "simpler") hamburgers At the risk of derailing the thread, that would be the lesson I wish people would take away from that example. Criticizing fast food like that is dumb signalling IMO; McDonald's!hamburger != homemade!hamburger. It's an entirely different product sharing the same name and some of the ingredients. It tastes different, and has a different form f…
Plenty of places can get me a significantly better burger in under a minute. McDonald's is not high up inside its category .
The fries are great though. Sometimes I go there for the fries, and occasionally I get a burger on the side
Re: Please do not attempt to simplify this code
#238Earlier quoted context omitted.
Well, let's consider an example from this very code: // The binding is two-step process. PV.Spec.ClaimRef is modified first and // PVC.Spec.VolumeName second. At any point of this transaction, the PV or PVC // can be modified by user or other controller or completely deleted. Also, // two (or more) controllers may try to bind different volumes to different // claims at the same time. The controller must recover from…
My comment was directed to the OP's question of comment:code ratio in general, not in this exact circumstance. Additionally, in no way am I advocating for no comments, that's obviously not possible (like your example). Comments are useful, even necessary, for code that might have an otherwise confusing logic to them. I've seen plenty of code with documentation for a method with nothing more than: /** * Bills the user…
no, in case like this, it's mostly for documentation generation. though you may find it useless.
Re: Please do not attempt to simplify this code
#239Reminds me of the classic SICP quip:
"Programs must be written for people to read, and only incidentally for machines to execute."
Re: Please do not attempt to simplify this code
#240Earlier quoted context omitted.
> probably a hell of a lot easier to maintain and manage than splitting the logic up among tens or hundreds of files I'm only halfway through John Ousterhout's book Philosophy of Software Design but I think it agrees with you on this -- that smallness-of-file or smallness-of-function is not a target to shoot for because it prevents the things you build from being deep. That you should strive to build modules which ha…
I just finished his book yesterday; he has a lot to say about size and comments. For size, your summary is spot-on. I'd only add that he notes overeager splitting of methods and classes makes code involved in a particular abstraction to be no longer in one place, leading developers to constantly jump around files, which makes it more difficult to understand the code and increases the chances of making bugs. As for co…
I really enjoyed the Ousterhout book as well. It's my current user manual for reviewing pull requests.
Putting my PR reviewer hat on I would say the code in question would pass muster if it were relatively stable so you would not be constantly redoing the comments. I love nice comments but Golang does not give you much help keeping them in sync.
(Weirdly enough I'm working on a PR for persistent volume documentation at VMware today so the code is very apropos.)