Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Please do not attempt to simplify this code

github.com

Post reply on HN
Print view View original

151–160 of 647 posts

Re: Please do not attempt to simplify this code

#151

Post by lozenge »

TeMPOraL wrote: post #138
Karrot_Kream wrote: post #86

Earlier quoted context omitted.

There's a lot of talk about comments becoming stale and code being self documenting in the replies which makes me wonder: do people genuinely not read comments and just made code changes without updating comments? And do reviewers not look at the context of the surrounding code and just let commits in? What's the point of having code reviews then?

I'd say it's mostly a meme. If a code review doesn't involve reviewing comments around the modified code, it's a bug in the process. Elsewhere in this thread Ousterhout's book is mentioned; I like his advice about always placing comments in the most obvious places and as close to the code they affect as possible. This way, you can't miss them, and and it's hard to forget to update them.

Everyone can have comment blindness to some extent, but I've worked with two people who auto-collapsed docstrings and didn't read and hence update comments, which is enough (one person writing code without updating comments/docstrings and one person inadequately reviewing). Sure, the problem only appears in a bit of the code, but it means people stop trusting all the comments.

Re: Please do not attempt to simplify this code

#152

Post by emn13 »

macintux wrote: post #105
emn13 wrote: post #77

Earlier quoted context omitted.

But what if comments pertain to unexpected states the system as a whole can be in? Kind of the whole problem is when there are weird corner cases going on that straddle function boundaries. I'm not saying that's a good thing; mind you - but nor is it always trivially avoidable, especially if code needs to be concurrency and/or exception safe -- or in general whenever the statements your function consists of have surp…

Agreed, but that’s why I prefer writing code that has no global state. In Erlang, for example, it’s unusual (and the language lends itself via pattern matching to having short function clauses). It gets a little tiresome threading the relevant state to every function that needs it, but it’s worthwhile in the end.

Even a pure function has "state" - namely its (arbitrarily complex) inputs. But sure, it's a little less of a landmine.

The fundamental issue remains that sometimes your knowledge about that state (whether the classical kind or a proper parameter) can be complex and dependent on what happened elsewhere, especially if the codebase your in was grown into that situation, and not designed like that per-se. A comprehensible set of preconditions and postconditions isn't always a luxury you have, certainly not at first.

Re: Please do not attempt to simplify this code

#154

Post by lmm »

endymi0n wrote: post #50
dajonker wrote: post #16

"it became clear that we needed to ensure that every single condition was handled and accounted for in the code" This is a feature of several (mostly functional) programming languages, e.g. Haskell. Fun to see that often people figure out that these types of concepts are a smart way to write your code. Too bad it usually means many people reinvent the wheel instead of learning about computer science history and other…

I know a business coach who regularly asks his audience "Who here makes better burgers than McDonalds?". When half the audience raises their hand, he asks them why they don't outsell this giant company. Functional programming advocats, especially for the "pure" ones like Haskell, always strike me as odd. It seems that all the beauty of those languages make people obsess over that beauty and purity while keeping them…

> Meanwhile, people with simpler languages like Go just get stuff done that is useful and makes people happy.

Kubernetes' reputation is just the opposite: that far from being a simple and useful thing, it's an overengineered, overcomplicated solution to a self-inflicted problem (deploying a distributed monolith).

> But the closest I ever came to using a functionally developed product was RabbitMQ (written in Erlang). That one was _such_ a pain to use and operate — must have been the developers still dreaming in the purity of its code instead of writing some installation docs. I moved on to Kafka later and didn't regret it a minute.

Erm, Kakfa was developed in Scala, whose advocates far more of a reputation for purist pontification than Erlang developers do. Maybe all that beauty and purity is actually good for something?

Re: Please do not attempt to simplify this code

#155

Post by thefounder »

hueving wrote: post #85
Klathmon wrote: post #21

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

This is what a lot of Go code looks like. This "space shuttle" code honestly isn't much more verbose than most Go code I interact with. The main difference is they have more comments here.

"else/else if" is not used very often in Go.

Re: Please do not attempt to simplify this code

#156

Post by emn13 »

TeMPOraL wrote: post #145
emn13 wrote: post #77

Earlier quoted context omitted.

But what if comments pertain to unexpected states the system as a whole can be in? Kind of the whole problem is when there are weird corner cases going on that straddle function boundaries. I'm not saying that's a good thing; mind you - but nor is it always trivially avoidable, especially if code needs to be concurrency and/or exception safe -- or in general whenever the statements your function consists of have surp…

> Kind of the whole problem is when there are weird corner cases going on that straddle function boundaries. If the problem has "hub and spokes" topology, i.e. it's relevant to multiple places in code that all reference a single location, put a comment describing the issue in that single location, and everywhere else put a comment with a reference. //Warning. See comment in [that location]. If there's no single best…

Centralized comment references sounds like a good+simple idea - I'll try to remember it, and hope I never have to of course ;-).

Re: Please do not attempt to simplify this code

#157

Post by geofft »

kraftman wrote: post #146
geofft wrote: post #115

Earlier quoted context omitted.

I'm not sure that reducing the comments-to-code ratio by increasing the complexity of the code really helps anything. You've made the code more generic for what you currently think future changes are going to look like, which may or may not be accurate. And in the process you've split dateIsOnLeapDay and convertLeapDayToPreviousDay into separate functions, so if someone is tracking down a bug in line 10, they need to…

> I'm not sure that reducing the comments-to-code ratio by increasing the complexity of the code really helps anything. More lines doesn't mean more complex, it's the same logic just the logic is named now and more reusable. It's possibly not the best example, as the logic is minimal, but when the logic becomes more complex, wrapping it and naming it becomes very powerful. We're creatures of abstraction. > You've mad…

> I'd argue that I've reduced the number of reasons the code has to change, which should be a goal while programming. If we change how we calculate a leap day, we don't touch how we modify a leap day, which means we're less likely to cause adverse side effects.

Maybe this is just different instincts/experience and I'm not saying you're wrong, but my feeling here is that you do actually want to change them at the same time. Suppose we decide instead of adding a day to February 29, we keep the months the same and add a festival day at the end of every fourth year, numbered 13/1. Then modifying the festival day to 13/0 is wrong - the day before 13/1 is now 12/31.

If you have one function for "fix leap days for reporting purposes" then you're fine, and you've set the abstraction in a good place (or at least good for my example case, I will totally concede there are other examples!). When you edit the is-it-the-leap-day line of code, you'll see the subtract-one line of code directly below, and if you forget, your reviewers are likely to notice. And you haven't really made things noticeably worse for the case where the customer says "Actually we need February 29 rounded to March 1, instead", it's not distracting to have that line of code above where you are (and if anything it's useful to have that comment, so that if this is a different customer asking you realize that you need to not break expectations for your first customer).

I am something of a skeptic of reusing very short pieces of code - for instance, my team's own codebase has a poorly-designed function for calling a subprocess and swallowing certain types of errors from a very specific command, and in a code review I had to tell someone to just use subprocess.check_output(), which does the same thing but without the modified behavior which they probably didn't want. Abstraction makes sense when there is a meaningful concept to abstract. (Similarly, I am also very much not a fan of getters and setters; I think most people are better off with a structure with public fields, because I have very rarely seen it be useful to convert a trivial getter/setter to a non-trivial one without bothering to look at how callers use it, and it is useful to rename the field and see which code fails to compile / no longer passes tests now.)

Re: Please do not attempt to simplify this code

#158

Post by ascar »

EB66 wrote: post #91
Klathmon wrote: post #21

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. 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 completely agree. For code that is unavoidably complex, I love this style too. I am all for code that is concise and whose syntax/naming is expressive, but sometimes comments are necessary to clearly spell out the logic or business use case. Expressive code can only go so far. Well-crafted comments significantly reduce the amount of time required for other developers to dive in and become productive with an unfamil…

Outdated comments that explain the business use case or purpose are still better than no comments. It gives you background information how the code evolved or what it was supposed to do.

It's probably because reading comments only is worse than reading code without comments, that some devs developed an aversion towards outdated comments and thus comments in general.

Comments are additional information and no source of truth, always take them as that and read code and comments.

Re: Please do not attempt to simplify this code

#159

Post by icebraining »

vvanders wrote: post #56
josteink wrote: post #35

Earlier quoted context omitted.

> do other HNers also feel that a high comment:code ratio probably indicates quality? I consider it a big risk of errors. When some code is changed, will all related comments be rewritten too? I doubt it. And then you end up with a codebase which indicate A but comments which clearly spell out B, and you as a maintainer have no idea what to believe. DRY. Don’t repeat yourself. The comments should not double up for th…

I've seen horrible inheritance/convoluted refactors done in pursuit of DRY. I'm a bigger fan of WET(Write Everything Twice). Usually the first iteration of a component you don't understand enough of the domain space to get the abstractions right. So use that first attempt to explore the issues/problems/corner cases. Once well understood, rewrite it into something concise and well abstracted. I've also find that if yo…

Which is the first iteration, the code or the comment?

Re: Please do not attempt to simplify this code

#160

Post by TeMPOraL »

lozenge wrote: post #151
TeMPOraL wrote: post #138

Earlier quoted context omitted.

I'd say it's mostly a meme. If a code review doesn't involve reviewing comments around the modified code, it's a bug in the process. Elsewhere in this thread Ousterhout's book is mentioned; I like his advice about always placing comments in the most obvious places and as close to the code they affect as possible. This way, you can't miss them, and and it's hard to forget to update them.

Everyone can have comment blindness to some extent, but I've worked with two people who auto-collapsed docstrings and didn't read and hence update comments, which is enough (one person writing code without updating comments/docstrings and one person inadequately reviewing). Sure, the problem only appears in a bit of the code, but it means people stop trusting all the comments.

> auto-collapsed docstrings

Woah, that sounds like a pretty dumb feature. Auto-collapsing whole functions is useful, but auto-collapsing docstrings sounds like a recipe for disaster. People write docstrings and inline comments for a reason.

Post reply on HN