The "what" vs "why" distinction breaks down when your code encodes domain knowledge that readers can't infer from context.
I build accounting software and half my "what" comments are actually explaining business rules that would be impenetrable otherwise. Something like:
// Bank transfers appear as two transactions - a debit here and credit elsewhere
// We match them by looking for equal-opposite amounts within a 3-day window
That's explaining "what" but also implicitly "why" - because that's how double-entry works and that's the tolerance banks allow for settlement delays. You can't really extract that into a method name without it becoming absurd.
The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.
> That's explaining "what" but also implicitly "why" - because that's how double-entry works and that's the tolerance banks allow for settlement delays. You can't really extract that into a method name without it becoming absurd.
That's why I've also started to explicitly decompose constants if possible. Something like `ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours`. Sure, this takes 4 constants and is longer to type than "28 hours".
However, it communicates how I think about this, how these 28 hours come about and how to tweak them if the alerting is being annoying: It's easy to guess that you'd bump that to 29 hours if it throws false positives. But like this, you could see that the backup is supposed to take, say, 3 hours - except now it takes 4 due to growth of the system. So like this, you can now apply an "obviously correct" fix of bumping up the approximate backup duration based on monitoring data.
I don't necessarily disagree with providing context, but my concern is that comments eventually lie. If the business rule evolves (say the window moves to 5 days) the comment becomes a liability the moment someone updates the code but forgets the prose.
The comment also leaves me with more questions: how do you handle multiple identical amounts in that window? I would still have to read the implementation to be sure.
I would prefer encoding this in an Uncle Bob style test. It acts as living documentation that cannot get out of sync with the code and explains the why through execution. For example:
> I don't necessarily disagree with providing context, but my concern is that comments eventually lie. If the business rule evolves (say the window moves to 5 days) the comment becomes a liability the moment someone updates the code but forgets the prose.
Yes, comments can drift out of sync with the code. That doesn't mean the comments were a bad idea; it just means the programmer didn't update the comments when they should have.
Similarly, even with zero comments, variable names can drift out of sync with their intended purpose, e.g. when they are repurposed to hold values that are different from what their name implies. Again, the solution is to rename the variable so it reflects its purpose (or possibly create another variable).
> This way, the 3 day rule is a hard requirement that fails the build if broken rather than a suggestion in a comment block.
What happens when a dev changes the test code, but fails to update the string "should_match_debit_with_credit_only_if_within_three_day_settlement_window"? That's effectively no different than comments being out of sync with the code.
This however misses an important point: 3 is not in our control. 3 in general is controlled by math-people, and that 3 in particular is probably in the hands of a legal/regulation department. That's a much more important information to highlight.
For example, at my last job, we shoved all constants managed by the balancing teams into a static class called BalancingTeam, to make it obvious that these values are not in our control. Tests, if (big-if) written, should revolve around the constants to not be brittle.
I like the idea of using a LegalConstants namespace or Code Owners to signal that we don't own those values.
However, I’d argue that being 'out of our control' is actually the main reason to test them. We treat these as acceptance tests. The goal isn't flexibility, it is safety. If a PR changes a regulated value, the test failure acts as a tripwire. It forces us to confirm with the PM (and the Jira ticket) that the change is intentional before merging. It catches what code structure alone might miss.
Until someone changes the test to be four days, or two, but doesn't update the test name.
Ultimately disciplined practice requirements rely on disciplined practices to succeed. You can move the place where the diligence needs to taken, but at the end the idea that comments can lose their meaning isn't that different to other non-functional, communicative elements also being subject to neglect. I don't mean to suggest that a longish test title wouldn't be more likely to be maintained, but even with that long name you are losing some context that is better expressed, person-to-person, using sentences or even paragraphs.
I had first hand experiences with this, oddly enough, also working on an accounting system (OK, ERP system... working the accounting bits). I was hired to bring certain deficient/neglected accounting methodologies up to a reasonable standard and implement a specialized inventory tracking capability. But the system was 20 years old and the original people behind the design and development had left the building. I have a pretty strong understanding of accounting and inventory management practices, and ERP norms in general, but there were instances where the what the system was doing didn't make sense, but there was no explanations (i.e. comments) as to why those choices had been taken. The accounting rules written in code were easier to deal with, but when we got to certain record life-cycle decisions, like the life-cycle evolution of credit memo transactions, the domain begins to shift from, "what does this mean from an accounting perspective", where generally accepted rules are likely to apply, to what did the designers of the application have in mind related to the controls of the application. Sure I could see what the code did and could duplicate it, but I couldn't understand it... not without doing a significant amount of detective work and speculation. The software developers that worked for the company that made this software were in the same boat: they had no idea why certain decisions were taken, if those decisions were right or wrong, or the consequences of changes... nor did they care really (part of the reason I was brought in). Even an out-of-date comment, one that didn't reflect the code has it evolved, would still have provided insight into the original intents. I know as well as you that code comments are often neglected as things change and I don't take it for granted that understanding the comments are sufficient for knowing what a piece of code does.... but understanding the mind of the author or last authors does have value and would have helped multiple times during that project.
When I see these kinds of discussions I'm always reminded of one of my favorite papers. By Peter Naur, "Programming As Theory Building" (https://pages.cs.wisc.edu/~remzi/Naur.pdf). In my mind, comments that were at least good and right at one time can give me a sense of the theory behind the application, even if they cannot tell me exactly how things work today.
we tend to talk about type composition, but values too are composed. this suggestion makes the composition clear and, imo, is strictly better that a hardcoded resulting value. even more so if we assign it using some blocks-with-returns feature, if the language has it: we can clarify that the value’s components are not used elsewhere and thus reduce complexity of the namespace. without such feature, I’m not sure complicating the namespace is worth it, and a comment may actually be better.
i personally prefer this kind of version — if i want to do the maths to work out tweaks i can, but i’m not forced to do maths in my head to know/tweak the end value
// a total of
// - backup interval = 24
// - approx backup duration = 2
// - “wiggle room” = 2
ageAlertThresholdHours = 28
yes lazy devs are lazy and won’t want to or just won’t update the comments (be pedantic in review :shrug:). it’s all trading one thing off with another at the end of the day.
edit — also your version forces me to read horizontal rather than vertical, which takes longer ime.
sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now.
const int backupIntervalHours = 24
const int approxBackupDurationHours = 2
const int wiggleRoomHours = 2
const int ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours;
static_assert(28 == ageAlertThresholdHours);
It's a shame more languages don't have static asserts... faking it with mismatched dimensions of array literal/duplicate keys in map literals is way too ugly and distracting from the intent.
No static assert needed, no need to pre-compute the total the first time, and no need to use identifiers like `approxBackupDurationHours`, the cognitive override about the possibility of colliding with other stuff that's in scope, or the superfluous/verbose variable declaration preamble.
I'm a believer in restricting the scope of definitions as much as possible, and like programming languages that allows creating local bindings for creating another.
For example:
local
val backupIntervalHours = 24
val approxBackupDurationHours = 2
val wiggleRoomHours = 2
in
val ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours
end
Then it's easier to document what components a constant is composed of using code without introducing unnecessary bindings in the scope of the relevant variable. Sure constants are just data, but the first questions that pops into my head when seeing something in unfamiliar code is "What is the purpose of this?", and the smaller the scope, the faster it can be discarded.
Mentally discarding a name still takes some amount of effort, even if local.
I often write things the way you have done it, for the simple reason that, when writing the code, maybe I feel that I might have more than one use for the constant, and I'm used to thinking algebraically.
Except, that I might make them global, at the top of a module. Why? Because they encode assumptions that might be useful to know at a glance.
And I probably wouldn't go back and remove the constants once they were named.
But I also have no problem with unnamed but commented constants like the ones in the comment you responded to.
> sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now.
That's all fine.
Just note that this was one of the easiest examples I could find. For example, for reasons out of my control, the individual network configuration on a linux host is positively nuts. The decision whether to deploy routes, static DNS servers and such depends on 3-5 facts about the datacenter and the provider it's on.
In such a case, it is more maintainable to separate the facts about the provider, or the thing we are looking at (e.g. "Does this provider allow us to configure routes in their DHCP server?", from the computation/decision making ("Can the system rely on the routes from the DHCP servers?"), and all of that from the actual action based off of the decision ("Deploy routes statically if DHCP provided routes are not correct").
What you describe really is describing the "why", not the "what".
The line between the two is not that blurry: assume your reader has total knowledge of programming, and no knowledge whatsoever of the outside world. Comments about what the code does to bits are the "what"; comments about how the code relates to the outside world are the "why". The rest is a matter of taste and judgment.
If you're writing a coding tutorial, you'll want to comment on the "what" indeed. Otherwise it will most likely end up being more distracting than useful, and sometimes even misleading. Exceptions exist, but by virtue of being exceptions there's no catch-all rule for them, so just use your judgment.
I almost (?) always advise against “what” comments. I have rarely (if ever?) encountered any cases where “what” comments didn’t have a better (and practical/cheap/easy enough) solution.
In my experience, when I review junior contributors’ code and see “what” comments, it’s usually caused by 1) bad naming, or 2) abstractions that don’t make sense, or 3) someone trying to reinvent maths but incorrectly, or 4) missing tests, or 5) issues with requirement gathering / problem specification, or 6) outright laziness where the contributor doesn’t want to take the time to really think things through, or 7) unclear and overcomplicated code, or… any number of similar things.
At the very least, any time you see a “what” comment, it’s valuable to take notice and try really hard to think about whether the problem the comment tries to solve has a better solution. Take it as a personal challenge.
While I agree, I think that's still incomplete. To me good comments have always been about "what is being done AND why".
Or to put it another way: to provide the necessary context for figuring out why a particular piece of code is written the way it's done. (Or what it's supposed to do.)
I agree with that as well, the "why" explains the business logic and that makes much more sense to me. Otherwise, I might end up explaining the algorithm (the "what") instead of the business behind it.
ime summaries/details of the “business what” are the most important part of “why the code exists/is written this way” — it’s the overarching problem that section of code aims to solve / needs to work around!
they can also be a good defence against newer seniors on the team refactoring willy nilly — can’t argue with business rules, but can argue over uncle bob. ;)
Does the caller of the method need to know the three days? Why? What if you update to FED Now and it changes?
It seems like the caller only needs to know that it’s checking within the window, which is then likely a constant (like SETTLEMENT_WINDOW_DAYS) that gives the context.
If you need to, you can add a comment to the constant linking to whatever documentation defines why three days.
I always use “what” comments for regular expressions, in addition I provide examples of before and after transformations so that future developers know immediately what’s going on without having to first stop, context-switch, and decipher hieroglyphs.
This is probably one of the best use cases for "what" comments... however in my opinion a much better way to go about this is to have example-based tests (and maybe a decent function name) serve as your documentation.
> The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.
When the name is long, it should be more than one function. Finding the transactions within a settlement window seems distinct from matching them up.
The comment about 3 days isn't necessary when "numDays" is a variable in the SettlementWindow class (or other relevant scope). I'd be a lot more comfortable reading code that makes this concept a proper abstraction rather than just seeing a comment about a hard-coded 3 somewhere. Magic values are bad and comments don't demystify anything if they get out of sync with the code.
> The "what" vs "why" distinction breaks down when your code encodes domain knowledge that readers can't infer from context.
Yes, there are multiples levels of knowledge, and the required level of commenting depends on the minimum knowledge expected of a reader in each of several dimensions.
In point of fact, the only thing that almost always can do without documentation is a different question, "How."
> and that's the tolerance banks allow for settlement delays
This is missing from your comment. While I'd probably be able to pick that up from context, I'd have to hold "this is an assumption I'm making" in my head, and test that assumption against all parts of the codebase I encounter until I become certain: this'd slow me down. I'd recommend including the "why" explicitly.
> The "what" vs "why" distinction breaks down when your code encodes domain knowledge that readers can't infer from context.
Your comment is a poor example because you unwittingly wrote "why" comments, not "what".
// We match them by looking for equal-opposite amounts within a 3-day window because bank transfers appear as two transactions - a debit here and credit elsewhere
If anything, you proved the importance of clarifying why things must be the way they are.
> The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.
You are also completely off in here as well, and I think that your comment shows a deeper misunderstanding of the topic. What you tried to frame as "Uncle Bob approach" actually means implementing your domain in a clear and unambiguous way so that both the "what" and the "why" are clear by looking at the code. Naming conventions are one aspect, but the biggest trait is specifying an interface that forces the caller no option other than doing the what's because of the why's.
Without looking too hard into it, whipping out something like this in a few seconds would match the intent
I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method). This was a thing for a while but anyone who's tried to fix bugs in a codebase like that knows exactly what this article is talking about. It's a constant frustration of pressing the "go to definition" key over and over, and going back and forth between separate pieces that run in sequence.
I don't know how that book ever got as big as it did, all you have to do is try it to know that it's very annoying and does not help readability at all.
and one which I highly recommend and which markedly improved my code --- the other book made me question my boss's competence when it showed up on his desk, but then it was placed under a monitor as a riser which reflected his opinion of it....
That entire conversation on comments is just wildly insane. Uncle Bob outright admits that he couldn't understand the code he had written when he looked back on it for the discussion, which should be an automatic failure. But he tries to justify the failure as merely the algorithm just being sooooo complex there's no way it can be done simply. (Which, compared to the numerics routines I've been staring out, no, this is among the easiest kind of algorithm to understand)
That's a really interesting read. I felt myself being closer to John about the small method part, but closer to UB for the TDD part, even if in both cases I was somewhere inbetween.
At the very least, you convinced me to add John's book to my ever-growing reading list.
One of the most infuriating categories of engineers to work with is the one who's always citing books in code review. It's effectively effort amplification as a defense mechanism, now instead of having a discussion with you I have to go read a book first. No thanks.
I do not give a shit that this practice is in a book written by some well respected whoever, if you can't explain why you think it applies here then I'm not going to approve your PR.
Yeah, and any of these philosophies are always terrible when you take them to their limit. The ideas are always good in principle and built on a nugget of truth, it's when people take it as gospel I have a problem. If they just read the book and drew inspiration for alternative, possibly better, coding styles and could argue their case that's unequivocally good.
It's like with goto. Goto is useful and readable in quite a few situations but people will write arrow like if/else tree with 8 levels of indentation just to avoid it because someone somewhere said goto is evil.
Funny how my Python code doesn't have those arrow issues. In C code, I understand some standard idioms, but I haven't really ever seen a goto I liked. (Those few people who are trying to outsmart the compiler would make a better impression on me by just showing the assembly.)
IMX, people mainly defend goto in C because of memory management and other forms of resource-acquisition/cleanup problems. But really it comes across to me that they just don't want to pay more function-call overhead (risk the compiler not inlining things). Otherwise you can easily have patterns like:
int get_resources_and_do_thing() {
RESOURCE_A* a = acquire_a();
int result = a ? get_other_resource_and_do_thing(a) : -1;
cleanup_a(a);
return result;
}
int get_other_resource_and_do_thing(RESOURCE_A* a) {
RESOURCE_B* b = acquire_b();
int result = b ? do_thing_with(a, b) : -2;
cleanup_b(b);
return result;
}
(I prefer for handling NULL to be the cleanup function's responsibility, as with `free()`.)
Maybe sometimes you'd inline the two acquisitions; since all the business logic is elsewhere (in `do_thing_with`), the cleanup stuff is simple enough that you don't really benefit from using `goto` to express it.
In the really interesting cases, `do_thing_with` could be a passed-in function pointer:
int get_resources_and_do(int(*thing_to_do)(RESOURCE_A*, RESOURCE_B*)) {
RESOURCE_A* a;
RESOURCE_B* b;
int result;
a = acquire_a();
if (!a) return -1;
b = acquire_b();
if (!b) { cleanup_a(a); return -2; }
result = thing_to_do(a, b);
cleanup_b(b); cleanup_a(a);
return result;
}
And then you only write that pattern once for all the functions that need the resources.
Of course, this is a contrived example, but the common uses I've seen do seem to be fairly similar. Yeah, people sometimes don't like this kind of pattern because `cleanup_a` appears twice — so don't go crazy with it. But I really think that `result = 2; goto a_cleanup;` (and introducing that label) is not better than `cleanup_a(a); return 2;`. Only at three or four steps of resource acquisition does that really save any effort, and that's a code smell anyway.
(And, of course, in C++ you get all the nice RAII idioms instead.)
You're assuming function calls or other constructs are more readable and better programming. I don't agree. Having a clear clean-up or common return block is a good readable pattern that puts all the logic right there in one place.
Jumping out of the loop with a goto is also more readable than what Python has to offer. Refactoring things into functions just because you need to control the flow of the program is an anti pattern. Those functions add indirection and might never be reused. Why would you do that even if it was free performance wise?
This is why new low level languages offer alternatives to goto (defer, labelled break/continue, labelled switch/case) that cover most of the use cases.
Imo it's debatable if those are better and more readable than goto. Defer might be. Labelled break probably isn't although it doesn't matter that much.
Python meanwhile offers you adding more indirection, exceptions (wtf?) or flags (inefficient unrolling and additional noise instead of just goto ITEM_FOUND or something).
A colleague recently added a linter rule against nested ternary statements. OK, I can see how those can be confusing, and there's probably a reason why that rule is an option.
Then replaced a pretty simple one with an anonymous immediately invoked function that contained a switch statement with a return for each case.
I guess "anonymous IIFE" is the part that bothers you. If someone is nesting ternary expressions in order to distinguish three or more cases, I think the switch is generally going to be clearer. Writing `foo = ...` in each case, while it might seem redundant, is not really any worse than writing `return ...` in each case, sure. But I might very well use an explicit, separately written function if there's something obvious to call it. Just for the separation of concerns: working through the cases vs. doing something with the result of the case logic.
It just looked way more complex (and it's easy to miss the () at the end of the whole expression that makes it II). And the point of the rule was to make code more readable.
Basically it's a shame that Typescript doesn't have a switch-style construct that is an expression.
And that nowadays you can't make nested ternaries look obvious with formatting because automated formatters (that are great) undo it.
> I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method)
Alas, there's a lot of Go people who enjoy that kind of thing (flashback to when I was looking at an interface calling an interface calling an interface calling an interface through 8 files ... which ended up in basically "set this cipher key" and y'know, it could just have been at the top.)
Hardcore proponents of this style often incant 'DRY' and talk about reuse, but in most cases, this reuse seems to be much more made available in principle than found useful in practice.
There's also the "it makes testing easier because you can just swap in another interface and you don't need mocks" argument - sure but half of the stuff I find like this doesn't even have tests and you still tend to need mocks for a whole bunch of other cases anyway.
I wonder how hard it would be to build an IDE "lens" extension that would automatically show you a recursively inlined version of the function you're hovering over when feasible and e.g. shorter than 20 lines.
Great, that's exactly how I feel with any style that demands "each class in its own file" or "each function in its own file" or whatever. I'd rather have everything I need in front of my eyes as much as possible, rather than have it all over the place just to conform with an arbitrary requirement.
I said this at a company I worked and got made fun of because "it's so much more organized". My take away is that the average person has zero ability to think critically.
If those demands made any sense they would be enforced by the languages themselves. It's mostly a way of claiming to be productive by renaming constants and moving code around.
I can assure you that I am very serious and I do cut things up almost as finely as Uncle Bob suggests. Where others balk at the suggestion that a function or method should never expand past 20 or so lines, I struggle to imagine a way I could ever justify having something that long in my own code.
But I definitely don't go about it the same way. Mr. Martin honestly just doesn't seem very good at implementing his ideas and getting the benefits that he anticipates from them. I think the core of this is that he doesn't appreciate how complex it is to create a class, at all, in the first place. (Especially when it doesn't model anything coherent or intuitive. But as Jeffries' Sudoku experience shows, also when it mistakenly models an object from the problem domain that is not especially relevant to the solution domain.)
The bit about parameters is also nonsense; pulling state from an implicit this-object is clearly worse than having it explicitly passed in, and is only pretending to have reduced dependencies. Similarly, in terms of cleanliness, mutating the this-object's state is worse than mutating a parameter, which of course is worse than returning a value. It's the sort of thing that you do as a concession to optimization, in languages (like, not Haskell family) where you pay a steep cost for repeatedly creating similar objects that have a lot of state information in common but can't actually share it.
As for single-line functions, I've found that usually it's better to inline them on a separate line, and name the result. The name for that value is about as... valuable as a function name would be. But there are always exceptions, I feel.
As John Carmack said: "if a lot of operations are supposed to happen in a sequential fashion, their code should follow sequentially" (https://cbarrete.com/carmack.html).
A single method with a few lines is easy to read, like the processor reading a single cache line, while having to jump around between methods is distracting and slow, like the processor having to read various RAM locations.
Depending on the language you can also have very good reasons to have many lines, for example in Java a method can't return multiple primitive values, so if you want to stick to primitives for performances you inline it and use curly braces to limit the scope of its internals.
The Ruby ecosystem was particularly bad about "DRY"(vs WET) and indirection back in the day.
Things were pretty dire until Sandi Metz introduced Ruby developers to the rest of the programming world with "Practical Object-Oriented Design". I think that helped start a movement away from "clever", "artisanal", and "elegant" and towards more practicality that favors the future programmer.
Does anyone remember debugging Ruby code where lines in stack traces don't exist because the code was dynamically generated at run time to reduce boilerplate? Pepperidge Farm remembers.
I wonder how hard it would be to have an IDE extension that would automatically show you a recursively inlined version of the function you're hovering over when feasible and e.g. shorter than 20 lines.
Haskell (and OCaml I suppose two) are outliers though as one is supposed to have a small functions for single case. It's also super easy to find them and haskell-language-server can even suggest which functions you want based on signatures you have.
But in other languages I agree - it's abomination and actually hurt developers with lower working memory (e.g. neuroatypical ones).
It’s because maths are the ultimate abstraction. It’s timeless and corner cases (almost) fully understood. Ok maybe not, but at least relative to whatever JavaScript developers are reinventing for the thousand time.
It's an extremism to get a strong reaction, but the takeaway is that you should aim when possible to make the code understandable without comments, and that a good programmer can make code more understandable than a newbie with comments.
But of course understandeable code with comments simply has much more bandwidth of expression so it will get the best of both worlds.
I see writing commentless code like practicing playing piano only with your left hand, it's a showoff and you can get fascinatingly close to the original piece (See Godowsky's Chopin adaptations for the left hand), but of course when you are done showing off, you will play with both hands.
> where each line is extracted into its own method
Never heard of "that style of programming" before, and I certainly know that Uncle Bob never adviced people to break down their programs so each line has it's own method/function. Are you perhaps mixing this with someone else?
> Even a switch statement with only two cases is larger than I'd like a single block or function to be.
His advice that follows, to leverage polymorphism to avoid switch statements isn't bad per-se, but his reasoning, that 6 lines is too long, was a reflection of his desire to get every function as short as possible.
In his own words, ( page 34 ):
> [functions] should be small. They should be smaller than that. That is not an assertion I can justify.
He then advocates for functions to be 2-3 lines each.
> to leverage polymorphism to avoid switch statements [...] was a reflection of his desire to get every function as short as possible.
That's both true, but long way away from "every line should have it's own method", but I guess parent exaggerated for effect and I misunderstood them, I took it literally when I shouldn't.
Thank you! Taking that refactoring advice at face value when I still was quite junior led to me writing an immensely over-abstracted framework that bit me in the butt for years afterwards when trying to debug or add a new feature. Wasn't easy to unlearn...
Thank God you did. I hear of seniors who still do this shit with no real sense of style because they just assume that over-abstracting everything is always the correct thing to do. You're learning, not unlearning!
I feel like a complete weirdo when it comes to comments and variable names. I've never worked professionally as a coder, but I've been working with python and a bit of js for like 15 years now. I strongly believe that variable names should be long, and explain what they are, and that comments should be long, and explain what's happening.
I have no idea why people want to "save time" to write short comments and short variable names. I just CTRL+C, CTRL+V my variable name anyway. They compound on each other, and that ends up adding an unnecessary level of complexity, and the possibility for errors.
When I come back to a piece of complex code after a year or two, I'm very, very happy that I've used a variable name like "three_tuple_of_weight_radius_price" instead of "tuple" or "t".
> I have no idea why people want to "save time" to write short comments and short variable names
Really long variable names put extra space between the operators and calls, which makes it harder to grok the logical structure of the expression. When I write `a + b` in code, it's in that order because the language demands it, and I use languages with that syntax for reasons of familiarity; but really, the `+` is the most important thing there, so it shouldn't be buried way off to the right.
I don't like for variable names to mark type. `weight_radius_price` is fine (and usually much better than `wrp`, and definitely much better than `tuple` or `t`) but a) we know it's a 3-tuple because we can see the right-hand-side of the equals sign; b) the structure of the name already conventionally tells us to expect more or less that (although maybe a class or structure abstraction is missing?); c) the choice of a tuple is probably not really relevant in context as compared to other ways of sticking three atomic values together.
I think this response misses the point. Yes, if I’d not been worse at coding a decade ago, I wouldn’t be revisiting the code in the first place.
The point of the redundancy of very clear, descriptive variable names, is that the code is going to suck tomorrow, even if it is good today. Future me is always going to look down on today me. The difference is that I planing for that, so maybe I can help out future me by being verbose, even if it isn’t necessary for much of the code that ends up being fine.
When I have a nasty bug, I don’t want to waste a day on something that could have been clear if I’d just explained it properly.
I think it's a leftover before proper LSP's, if you had to type/paste everything, that would be fairly tedious, but now-a-days everyone uses LSP's with autocomplete that have all the variables in scope.
I do think very long names can hurt readability though so it's still a balancing act, but variables like "t" (often used in Haskell, ugh) is awful in my opinion.
I’m not sure what you mean. You’re not necessarily going to know the type of a variable just by reading a random section of code… especially in Python.
I absolutely going to add the type to the variable name if it’s a complex function. It’s just clearer.
The point is to not care about the type. If you see `weight, radius, price = ...`, then it generally isn't going to matter whether `...` is a tuple or a list (or more exotic possibilities). What matters is that you can iterate over it, and iterating over it gives exactly three results, and the first result represents a weight, etc.
If your weights need to be, say, a floating-point number of kilograms, then you establish that convention, and only mark intentional deviations from the convention (e.g. because you're doing an explicit conversion to format output). (Or you use a library like Pint to give more formality to it.)
The point is that you clearly aren't mutating it in the current context. Therefore, knowing that you could mutate it doesn't help you understand what the current code is doing, nor guide you away from mistakes (as you would have no reason to try to mutate it).
I operate a site where I have lists of things that have operations done on them all the time, occasionally I’ll load those series into memory as a tuple to save memory.
If I’m adding a feature to the site in 5 years, it’s going to be important to know if I’ve done that or not.
The bigger point I take from this is that the purpose of comments and good names are all attempts to help the developer grasp "the context of this code". The article uses "context switch" repeatedly, and in fact never uses the word "context" any other way. Since the author acknowledged they're starting a friendly flame war, I'll go ahead and add that the biggest problem with the example code is that it's object-oriented and mutable, which forces a sprawling context on the developers working with it.
When I read the replace() method, I was immediately confused because it has no arguments. stringToReplace and alreadyReplaced are properties of the class, so you must look elsewhere (meaning, outside of the function) for their definitions. You also don't know what other bits of the class are doing with those properties when you're not looking. Both of these facts inflate the context you have to carry around in your head.
Why is this a class? In the replace() method, there is a call to translate(symbolName). Why isn't there also a SymbolNameTranslator class with a translate() method? Who decided one was simple enough to use a function while the other warrants a class?
SymbolReplacer surely could also be done with a function. I understand that this is illustration code, so the usage and purpose is not clear (and the original Bob Martin article does not help). Is there a reason we want these half-replaced strings made available every time we call SymbolReplacer.replace()? If there is, we can get the same data by using a reduce function and returning all of the iterations in a list.
A plain, immutable function necessarily contains within it the entire scope of its behavior. It accepts and returns plain data that has no baggage attached to it about the data's purpose. It does one thing only.
One thing I learned from programming since the early 2000s, there is no such thing as one size fits all advice. You do what is best for future folks--as I like to call the unfortunate folks who would have to maintain the code I wrote--by providing them helpful hints (be it business rules, assumptions related to code/tech) along with as simply and clearly written code as possible (how do I know if my code is simple and easy to understand? Have a junior teammate review my code and have her/him leave comments wherever she has to spend more than 10-15 mins reading an area in the code).
I hope not of a lot of the future folks hate me for leaving them with ample context and clear/dead simple code.
In a very real way, a codebase is a conversation among developers. There's every chance the next guy will be less familiar with the code than you, who just did a deep dive and arrived at some critical, poorly documented line of code. There's an even better chance that person will be you, 6 months from now.
I also find that phrase super misleading. I've been using a different heuristic that seems to work better for me - "comments should add relevant information that is missing." This works against redundant comments but also isn't ambigous about what "why" means.
There might be a better one that also takes into account whether the code does something weird or unexpected for the reader (like the duplicate clear call from the article).
I like this framing, but might add to it: "comments should add relevant information that is missing and which can't easily be added by refactoring the code".
The article is about comments. But, more generally, I think the issue here is about naming things.
Names capture ideas. Only if we name something can we (or at least I) reason about it. The more clear and descriptive a name for something is, the less cognitive load is required to include the thing in that reasoning.
TFA's example that "weight" is a better variable name than "w" is because "weight" immediately has a meaning while use of "w" requires me to carry around the cumbersome "w is weight" whenever I see or think about "w".
Function names serve the same purpose as variable names but for operations instead of data.
Of course, with naming, context matters and defining functions adds lines of code which adds complexity. As does defining overly verbose variable names: "the_weight_of_the_red_ball" instead of "weight". So, some balance that takes into account the context is needed and perhaps there is some art in finding that balance.
Comments, then, provide a useful intermediate on a spectrum between function-heavy "Uncle Bob" style and function-less "stream of consciousness" style.
Explain "why not what" is good general advice. My further advice for comments is: even bad comments can be useful (unless they're from LLM output maybe...) therefore when in doubt, write a comment. Write it in your own words.
Had to add the last sentence for the circa 2020s developer experience. LLM comments are almost never useful since they're supposed to convey meaningful information to another human coder, anything your human brain can think of will probably be more helpful context.
I strongly disagree. If you're using something like Claude Code to generate the code, it has significant context about the task, which from my experience provides very useful (albeit overly verbose) comments. I sometimes edit/rewrite its comments (as I might with the code itself), but I would never ask it to generate uncommented code.
Bad comments aren’t just less helpful than possible, they’re often harmful.
Once you’ve hit a couple misleading comments in a code base (ie not updated, flatly wrong, or deeply confusing due to term misuse), the calculus swings to actively ignoring those lying lies and reading the code directly. And the kind of mind resorting to prose when struggling to clarify programming frequently struggles with both maintenance and clarity of prose.
I hear this argument as an excuse not to write comments (sometimes at all). Maybe I am just lucky but I have never had this issue as you've described in codebases, and if I did, certainly not to that extent where it became a memorable thorn in my side.
If there are no comments, you are reading the code (or some relatively far away document) for all understanding anyway. If there are inaccurate comments, worst case you're in the same boat except maybe proceeding with a bit more caution next comment you come across. I always ask of fellow engineers: why is it unduly difficult to also fix/change the comments as you alter the code they refer to? How and when to use comments is a balancing of trade-offs: potential maintenance burden in the future if next writers are lazy vs. opportunity to share critical context nearest its subject in a place where the reader is most likely to encounter it.
These examples could both be much better IMHO with a top comment block that describes the purpose of the functionality and shows good usage examples. Something like this below, and ideally using runnable doc comments to help keep the comment correctly explaining the code.
Replace symbol placeholders in the input string with translated values. Scan the string for symbol placeholders that use the format "$foo". The format uses a dollar sign, then optional ASCII letter, then optional word characters. Each recognized symbol is replaced with its corresponding value.
Symbols are only replaced if the symbol exists i.e. getSymbol(String) returns non-null, and the symbol has not already been replaced in this invocation.
Example:
- input = "Hello $name, welcome to $city!"
- output -> "Hello Alice, welcome to Boston!"
Return the string with symbol placeholders replaced.
I made a point here https://antirez.com/news/124 that comments are needed at the same time for different reasons, and different comments have differente semantical properties that can be classified in classes you very easily find again and again, even in very different code bases.
This is a great post and meshes with how I like to comment as well. I like to break the so called rules and get a bit dirty when it comes to writing code and comments. My opinion which you state, is to remove the effort from the reader in needing to figure things out a second, third, or n-th time.
Here is one I wrote just to talk about iterating a loop in reverse:
/*
* We iterate the v6 prefixes in reverse from longest prefix length
* to shortest. This is because the ipv6 address is a sequence of bytes,
* and we want to perturb the address iteratively to get the corresponding
* network address without making a copy for each perturbation as that
* would be expensive.
*
* For example take address: abcd:abcd:abcd:abcd:abcd:abcd:abcd:abcd.
*
* With masks /112, /64, /8 we want to create the following network addresses
* to lookup as follows:
*
* Lookup abcd:abcd:abcd:abcd:abcd:abcd:abcd:0000 in /112 bucket
* Lookup abcd:abcd:abcd:abcd:0000:0000:0000:0000 in /64 bucket
* Lookup abcd:0000:0000:0000:0000:0000:0000:0000 in /8 bucket
*
* In any other order aside from most specific to least, we'd have
* to create copies of the original address and apply the mask each
* time to get each network address; whereas in this case we can take
* the same address and clear lower bits to higher bits as we go from
* most specific to least specific masks without incurring any copies.
*/
for (auto it = m_v6_prefixes.crbegin(); it != m_v6_prefixes.crend(); ++it)
Or here is another for masking a v4 address, but also explaining why a uint64 is used (this is calculated in a hot loop [same as the previous comment example], so I felt it was imperative to explain what is going on as there is very little room otherwise to optimise):
for (const auto & [ mask_len, bucket ] : m_v4_prefixes)
{
/*
* Example:
*
* netmask 255.255.128.0 (/17) or 0xffff8000:
*
* 0xffffffff ffffffff << (32-17)
* --------------------------------
* 0xffffffff ffff8000 (shifted left 15)
*
* Applying against address 192.168.85.146 or 0xc0a85592:
*
* (converted to uint64_t due to implicit integer promotion)
*
* 0x00000000 c0a85592
* &
* 0xffffffff ffff8000
* -------------------
* 0x00000000 c0a80000
* -------------------
* 0xc0a80000 (after conversion to uint32_t causing
* lower 32-bit truncation)
*/
std::uint64_t mask = (~0ULL) << (32 - mask_len);
std::uint32_t network_addr = addr & mask;
const auto itt = bucket.find(network_addr);
Except your giant comment doesn't actually explain why it used uint64. Only place mentioning uint64 is integer promotion which only happens because you used 64bit integer, thus no explanation of why.
Was it done because shifting by amount equal or greater to integer width is undefined behavior? That would still not require storing result in 64bit mask, just shifting (~0ULL) would be enough. That would be a lot more valuable to explain than how bitwise AND works.
The first one also seems slightly sketchy but without knowing rest of details it's hard to be sure. IPV6 address is 128bits, that's 2 registers worth integers. Calculating base address would take 2 bitwise instruction. Cost of copying them in most cases would be negligible compared to doing the lookup in whatever containers you are searching resulting address. If you are storing it as dynamically allocated byte arrays (which would make copying non trivial) and processing it in such a hot loop where it matters, then seems like you have much bigger problems.
For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps.
Sorry, I didn't explain uint64 was used. I wrote this many years ago so my memory was foggy, but I went through a few iterations using uint32 only to use branches for the masks. This was the only branchless way I could come up with at the time after a few attempts. I think the example was more to demonstrate that the algorithm was correct and I wasn't going to unit test it at that scope.
As for the 128-bit addresses, we used boost ip::address_v6 to_bytes() as it appears there was no masking option.
For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps.
Ah apologies, too late now, should've mentioned it in the PR. But I expected it would ruffle some feathers, I don't care for conventions or other people's quibbles. As long as it improves understanding for the reader, regardless of how "redundant", then mission accomplished.
Sometimes I want to use comments because I'm doing something vaguely algorithmic, and I know some readers won't follow the code.
I'm trying to think of a good example, maybe something like a pointer window based function (off the top of my head)
(This isn't real code. Don't get hung up on it)
func DedupeStrings(ss []string) []string {
if len(ss) < 2 {
return ss
}
ss = strings.Sort(ss)
u := 1 // index of end of "uniques" set
for i := 1; i < len(ss); i++ {
// Consume until new value
if ss[i] == ss[i-1] {
continue
}
// Put new value in 'uniques' set
ss[u] = sorted[i]
u++
}
// Final state: all unique items are positioned
// left of 'u' index
return ss[:u]
}
People will quibble, but
- I'm not convinced you could change the variable names without harming clarity. Would a name like uniquesEndIndex really be any clearer? It adds noise to the code and still doesn't satisfy a confused reader
- I don't want to use function calls for documentation, eg putInUniques(). I'm doing it this way because I want it to run really quick.
I'll be honest, this code is easier to read for me without the comments. Also sorting feels like it's going to be slower than having some kind of set structure? You don't need ordering, just collocation of duplicates. If not or if it's a wash, that is also a good thing to comment. Also I'm not sure about the semantics of Go but it seems this mutates the argument AND returns a value, something I consider dangerous.
Otherwise I agree, people have a weird hang up about short variable names. Somehow not a problem in mathematics...
There's probably a better example. The point is sometimes the What needs explanation, and finding a better What isn't practical.
I have slightly unorthodox opinions about short variables. I used to hate them. Then I posted a question on one of the PL design forums - it might have been Reddit r/programminglanguages - why is there are history of single letter names for type variables? ie T, U, etc for generics. The answer I got back, was, sometimes you want code to focus on structure rather than identities. That stuck with me, because it helped me understand why so much C code (including Linux) code uses similar naming practices. Names can lie, and sometimes expressing the structure is the absolute critical thing.
Back when I programmed in Haskell, I also had a similar question about the extremely terse variable names that pop up everywhere. I'd wonder, why is this "x" and "xs" instead of "item" and "items" or "businessName" and "businessNames" or whatever. Eventually I found this (paraphrased) answer that made it all click:
The specificity or abstractness of a (variable) name relates to the values that it can hold. So when you have a very abstract function whose inputs can be of almost any type, naming those inputs in an overly-specific manner is an exact inverse of the failure of giving an overly generic to name highly constrained parameter.
All this said, I do agree with your original take on the comments. I much prefer having human-readable explanations inline with anyhow non-trivial code. If nothing else, they really make it easier to correctly fix the code if a bug is found much later.
What is ss supposed to mean? Also, I only know what "u" means because of your "uniques" comment.
Those comments also don't really help me quickly understand the code. I'd do a small doc comment along the lines of "Removes repeated elements from the supplied list, returning the remaining items in original order"
I agree this is easy enough to follow but I'd like to quibble about something else:
Comments should answer the question why you are not using some kind of hash set and do a single pass over the data and why it's OK to reorder the strings. One could reasonable expect that Dedupe shows first occurrences in order.
I agree wholeheartedly - I've fought against refactors like this for the same reason. In the end readability is king, and jumping through 6 methods instead of one decreases readability quite a bit.
I find that this makes skimming lisp code much easier, because I can usually skip reading the bindings and just read the function name and ultimate expression and usually get the gist very quickly.
You might wonder how this is different than the example you provided, and the answer is because you could sneakily intersperse anything you wanted between your imperative bindings (like a conditional return statement), so I actually have to read every line of your code, vs in a lisp `let` I know for a fact there is nothing sneaky going on.
I appreciate this way of programming - also, if I may, in the age of auto-complete I think it's okay to have verbose variable naming. Imho, it's perfectly fine to have quad, quadVelocity, dragMagnitude, etc.
I see this a lot in the wild, though - as an honest question (not trolling!) why do people still shorten their variable names in place of having a terse descriptor ?
I tend to have shorter names for function scoped variables. Wider the scope - more descriptive the name. Short names are good to have more concise code and more logic fitting into a line width.
Funny how even in this very verbose code (which is perfectly fine btw), there's still a need to use shorthands (quadVel). It's like completely explained code requires so many words, but we need to fit the code into a certain character window.
The "don't explain variable definitions with comments because people won't see the comments at usage sites" argument also seems obsolete in the face of modern(?) IDEs.
If I look through code and see a variable I don't know, I want to see its definition anyway, so I know the type, scope, initial value, etc. And it's trivially possible to do that with everything that has a "jump to definition" command.
While it leads to more things to read, which thus may
take time, I feel that the benefits of using comments
far outweighs the negative sides. This is even valid
when comments are outdated usually. To me, adjusting
and updating comments often was much easier and faster
than describing something de-novo.
In the ruby land this is quite problematic because many
do not use any comments. The result is a horrible code
base. Getting people who wrote that code to use comments
is often too late, as they already abandoned ruby in favour
of another language, so I never buy the cop-out explanation
of "the code is self-explanatory" - not even in ruby it is.
Everyone can write horrible code.
Love it. Particularly the "correct" refactoring at the end. The future maintainer will thank you. (BTW: that future maintainer might be you). To a first approximation one should write "local" code i.e. the bits that are important should be right in front of you in the editor. It shouldn't need too much navigation, with all the overhead of context switches, to understand what you're looking at.
This is tangential to the article's point, but that `replace` function is a complete WTF in a way both authors completely ignore. Because it replaces things in the entire string in a loop, it will translate symbols recursively or not depending on ordering. Imagine you have the following dictionary:
a=$b
b=oops
if your input string just has one of these, it will just be translated once as the programmer was probably expecting:
input: foo $a bar
output: foo $b bar
but if your input string first references $b later, then it will recursively translate $a.
input: foo $a bar $b
output: foo oops bar oops
Sometimes translating recursively is a bizarre behavior and possibly a security hole.
The sane thing would be to loop through building the output string, adding the replacement for each symbol as you go. Using String.replace and the alreadyReplaced map is just a bad idea. Also inefficient, as it and throws away strings and does a redundant search on each loop iteration.
Feels typical of this whole '90s-era culture of arguing over refactoring with Java design patterns and ornate styles without ever thinking about if the algorithm is any good.
Edit: also, consider $foo $foobar. It doesn't properly tokenize on replacement, so this will also be wrong.
> The sane thing would be to loop through building the output string, adding the replacement for each symbol as you go.
As follows:
// SYMBOL_REF should be a class-level static final to avoid recompiling on each call.
int pos = 0; // input[..pos] has been processed.
StringBuilder out = new StringBuilder(); // could also guess at length here.
Matcher m = SYMBOL_REF.matcher(input);
while (m.find()) {
String replacement = symbols.get(m.group(1));
if (replacement == null) {
continue; // no such symbol; keep literal `$foo`.
}
out.append(input, pos, m.start());
out.append(replacement);
pos = m.end();
}
out.append(input, pos, input.length());
(Apparently there's also now a Matcher.replaceAll one could use, but it's arguably cheating to outsource the loop to a method that probably didn't exist when the Uncle Bob version was written, and it's slightly less efficient in the "no such symbol" case.)
Coding style must serve the purpose of aiding understanding. If you have strong opinions about the coding style of `replace` but those opinions don't lead to recognition that the implementation was incorrect and inefficient, your opinions are bad and you should feel bad. Stop writing garbage books and blog posts!
Ignoring the git commit message strawman (those can include "what/why the change", not "what/why the code") and the Uncle Bob strawman, the final code block looks fine. But notice:
// translate will replace all instances; only need to run it once
This is a "why".
// Replace all symbols
This is a "what". It's better conveyed by improving the function name rather than by a comment:
String replaceAllSymbols() {
Ultimately this article buttresses conventional wisdom about comments.
Also note, that after Uncle Bob's refactoring, you now have six additional functions floating around, which may or may not make sense outside of the context of their original caller. This can make the API surface of your class (even if it's just the internal one) harder to grasp and it invites other devs to reuse code that was never intended for reuse, creating unintended couplings.
That's mainly a problem for languages that don't allow nested functions though. But in Java, I am sceptical of excessive function creation for the sake of self-documenting code, unless maybe it's in the context of a command pattern, where the whole class only has one obvious function anyway.
This is a great practice. I used to do it when writing complex algorithms, I'd do pseudo-code comments to outline functionality, then basically implement the comments line by line.
Now I sometimes use this practice when working with agents, if I need something done just a certain way. It's time consuming, but it produces good results.
As I am involved in more low level stuff, I prefer to read the source than the man pages, and I am very happy with people overcommenting their code as a user of e.g. a lib. On the other hand, it is unbearable to me to see comments on a codebase I am working on. Fortunately, emacs show/hide comments exists, so I find myself overcommenting things.
We need to differentiate between a "What this is" comment and a "What this does" comment. "What this does" is a lot closer to "Why" than it is to "What this is". I would hope "What this is" is rarely needed but "What this does" can certainly be helpful pretty often.
> // translate will replace all instances; only need to run it once
That comment is not helpful, because it's wrong. The translate() function just is some sort of lookup. It doesn't replace anything. It's the stringToReplace.replace() call that replaces all instances.
I'm a data scientist and a lot of my R code are dplyr-chains a la data |> select(features) |> filter(low_quality) |> mutate(feature=...).
It just saves time to comment on what those chains do instead of having go through them every time I want to change something.
Comments should explain everything, but via links. Large comments cause context rot, so keep your comments tight and focused, and provide links to details as needed.
Agreed... IFF you're linking to something immutable (Jira tickets probably qualify in many enterprise scenarios). Otherwise linkrot is at least as big a problem. Meaningful commit messages surfaced in-IDE via git blame can serve a similar purpose.
My favorite genre of comments: it warns you not to touch it, and if you do, you must add to the list under the comment how many hours you wasted and got nowhere.
I agree with the OP, but, unfortunately, posts like this generate more heat than light. Like a lot of things, what and how to comment code comes down to “do the right thing,” not a list of rules, where everyone will always find counter examples. Do whatever you need to communicate effectively with “the next guy,” who could very well be you.
Should stick a (2017) on there as this is almost a decade old.
Comments should explain. If it’s why, it’s why. If it’s what, it’s what. The point of comments is to explain, to the reader/editor, what the intent of the code is and how you chose to approach it. That’s it. No need to fight over what, why, how, who, etc.
If you have domain knowledge that helps support the code, maybe a link to further reading, by all means add it to comments please!
I used to do that (and it's part of why I put a double line between functions and classes). But over time I started to feel like those were the points where I should just refactor. (If the function can't be cleanly cut at those joints, in turn, I take that as a sign that the logic needs to be disentangled first.)
.Net languages have ‘regions’ that can collapse & nest, providing a high level narrative of file organization (and, in practice, display most files in a clean collapsed view).
I really do not like working with uncle bob hardliners
There have been so many times where I have commented _why_ I think some uncle bob-ism made the code unclear and the response is always:
> *Sends link to Clean Code, maybe you don't know about this?
No, I do, and I am allowed to disagree with it. To which they always clutch their pearls "How do you think you know better than uncle bob!?", this is a Well Established Pattern TM.
I don't think I know better than Uncle Bob, but I don't think Uncle Bob works on this codebase nearly as much as you or I.
Isn’t the purpose of comments to make code understandable?
If that needs a why it’s a why-comment.
If it needs a what it’s a what-comment. Especially if clever programming tricks are used. 6 month later you already forgot what the trick is and how it works.
Yes, but some people have a Jihad against "what" comments - believing these should be elevated into the structure of the code itself, eg veryVerbosePainfullySpelledOutVariableNames or thisFunctionNameExplainsAWholeDomainConcept()
Yes, but there are other ways to make it more understandable (like good names, idiomatic code, units that are neither too long nor too short) that are often preferable, because comments always have a danger of going out of sync with the code.
The "why" is the part of the explanation that can't be deduced from the code.
I think the benefits of either clean code (sepearate methods) and commenting "what" are of different kind:
- clean code one, for me it just reads easier, specific bits of the larger operation are not surrounded with noise coming from other bits like it is in the commenting "what" one. I can focus more on each step, and that can make it easier to spot a bug or to refactor/adjust/extend, as now I'm more like on a given page of Lego set instructions
- the commenting of "what" one - yeah, this obviously makes it quicker (but probably not easier) to see the larger picture. It has benefits, but I believe this helps more hacker/scripter kind of people than programmers
IMO the example shows exactly that splitting code in smaller pieces is way better than just commenting it.
It makes it easier for dev's brain to parse the code e.g. to understand what code really does, while fattier but commented version makes it harder but tries to replace it with information about original coder's intentions. Which is maybe important too but not as important as code itself.
Not to forget that it's too easy to change code and leave comments obsolete.
I do not agree; I think the example that is not split is clearer and does not need comments to explain it (the variable names are helpful, although even if local variables use only one letter it still seems like clearly enough; for variables that are not local to that function, it does help to have a more descriptive names to understand them better). Comments and splitting can both be helpful in some circumstances, but neither is helpful for this one.
Splitting example is way too much indirection, but capturing what the code does in the code itself is a preference for me. In any high level language don't know why the middleground wasn't explored:
Technically this performs worse because you lose short-circuiting, but in performance-sensitive contexts code styling is less a concern anyways. And I also wouldn't rely on short-cutting alone to avoid a really expensive operation or side-effect: at some point someone will fail to notice it.
The compiler would have to determine that these are pure calls which I wouldn't rely on if performance actually matters
I just tested a recent gcc at -O2 with a contrived example using strings in an unordered_set: a look-up always occurs if not relying on short-circuiting
I like your version, and it's certainly possible to split too much without any practical result just for the dogma. But wrt the particular example I can see what's going on with a glance over the split part, while I have to focus at the commented one. Comments themselves can be helpful, but they can also be misleading cause code and coder's thoughts are not guaranteed to be in harmony all the time.
I'm all against commenting gatekeeping. IMO this is an anti-pattern.
Even in stupidest cases, like:
// add two and two
let four = two + two
The rationale for this is that if you start to mute developer on case of "this is not comment-worthy" you start to actually losing context for the codebase. Sure, maybe "add two and two" is not contextful enough, but "add same account twice" might be signal for appropriate code.
Maybe that's me, but I rarely saw teams which over-document, under-documenting is usually the case. So if we ever meet in professional environment - comment away. Worst case scenario - I'll skim over it.
Claude Code in particular seems to use very few redundant comments. That or it's just better at obeying the standing instruction I give it to not create them, something other assistants seem to blithely ignore.
I have changed my commenting style now, since the rise of LLMs.
I used to comment in a similar way to claude/chatgpt, as in both the what and why. (although the why in LLMs is often lost. ) I used to comment as if it was for someone who didn't know the libraries very well. (ie me in two years time. Documentation at the previous place was utterly shite, so it was effectively a journal of discovery)
However, my commenting style is both what and why. My variable names can be longer than my FANG peers, mainly because I know that English not being a first language means that half arsed abbreviations are difficult to parse.
But effort is placed on not using fancy features, python mostly, so avoiding lambdas and other "syntactic sugar", unless they absolutely make sense. If I do use them, i have a comment saying _why_ I'm doing it.
Some of my colleagues were dead against any kind of commenting. "my code should be readable enough without it". They had the luxury of working on one bit for a few months, then throwing it away.
“What” comments can be quite nice to quickly parse through code.
But I don’t think they’re worth it. My issue with “what” comments is they’re brittle and can easily go out of sync with the code they’re describing. There’s no automation like type checking or unit tests that can enforce that comments stay accurate to the code they describe. Maybe LLMs can check this but in my experience they miss a lot of things.
When “what” comments go out of sync with the code, they spread misinformation and confusion. This is worse than no comments at all so I don’t think they’re worth it.
“Why” comments tend to be more stable and orthogonal to the implementation.
People (and I'm one of them) usually say why instead of what because in general the what can be understood if you can read code. But obviously no hard rules, if I write something twisted because I had no idea how to do it better or had the time, then I'll write what it does. It's not perfect, but nothing is.
Same goes for comments vs commit messages. It's a fact comments get outdated and then you have an even bigger problem whereas a commit message is always correct at the time it was made. But obviously again, no hard rules. Sometimes I feel it's better to write a comment, e.g. if it's something that is really important and won't change a lot.
Here's a thought, and I'm just spitballin' here: maybe they could do both? Sure, writing an encyclopedic tome about three lines of code is probably a bit out of bounds, but overexplaining is only bad when it's your wife or girlfriend. It might be a good idea to be absolutely clear about things.
In my early days I read a lot of "you should" "this is wrong". But code is an expressive medium, you can do things one way, or do it the other, you can write "amountOfEmployees" or you can write "ne" with an "#amount of employees" comment, either way is absolutely fine and you can use whichever depending on your priorities, tradeoffs or even your mood.
Also I used to obsess over code, (and there's a lot of material that obsesses about code), but after you become profficient at it, there's a cap on the returns of investing time into your codebase, and you start to focus on the product itself. There's not much difference between a good codebase and a marvelously polished codebase, so you might as well use the extra focuse on going from a bad UX to a neutral UX or whatever improvement you can make to the product.
There are five canonical questions: What, When, Where, Why, and How.
"When" is occasionally a good question to answer in a comment, e.g. for an interrupt handler, and "Where" is also occasionally a good thing to answer, e.g. "this code is only executed on ARM systems."
The other three questions typically form a hierarchy: Why -> What -> How.
A simplistic google shows that "code comments" are next to "what" and "how" at about the same frequency as they are next to "why" and "what."
This makes some amount of sense, when you consider the usual context. "Why" is often an (assumed) obvious unstated business reason, "What" is a thing done in support of that reason, and "How" is the mechanics of doing the thing.
But with multiple levels of abstraction, _maybe_ the "What" inside the hierarchy remains a "What" to the level above it, but becomes a "Why" to the next level of "What" in the hierarchy. Or maybe the "How" at the end of the hierarchy remains a "How" to the level above it but becomes a "What" to a new "How" level below it.
Is it:
Why -> What/Why -> What/Why -> What/Why -> What -> How
or
Why -> What -> How/What -> How/What -> How/What -> How
In many cases the intermediate nodes in this graph could be legitimately viewed as either What/Why or as How/What, depending on your viewpoint, which could partly depend on which code you read first.
In any case, there are a few hierarchies with final "Hows" that absolutely beg for comments (Carmack's famous inverse square root comes to mind) but in most problem domains that don't involve knowledge across system boundaries (e.g. cache optimization, atomic operations, etc.), the final "How" is almost always adequately explained by the code, _if_ the reader understands the immediately preceding "What."
If I see a function "BackupDatabase()" then I'm pretty sure I already know both "Why" and "What" at the highest levels. "How" I backup the database might be obvious once I am reading inside the function, or the code might be opaque enough that a particular set of lines requires explanation. You could view that explanation as part of "How" the database is backed up, or you could view that explanation as "What" the next few lines of code are doing.
Again, this viewpoint might even partly depend on where you started. If you are dumped inside the function by a debugger, your question might be "What the heck is this code doing here?" but if you are reading the code semi-linearly in an editor, you might wonder "How is BackupDatabase() implemented?"
No, you can't always do that. We have workarounds for platform bugs that were even fixed, because we get users with old devices that can't upgrade. You cannot fork a phone of a random person on the other side of the world. Once a platform bug is out, it can stay out in the wild for a very long time.
Care to elaborate? It's not obvious to me why/how that would happen. In fact, my experience so far tells me that code with tons of "why" comments (and possibly some "what"s as well) makes LLMs less likely to break stuff.
I build accounting software and half my "what" comments are actually explaining business rules that would be impenetrable otherwise. Something like:
That's explaining "what" but also implicitly "why" - because that's how double-entry works and that's the tolerance banks allow for settlement delays. You can't really extract that into a method name without it becoming absurd.The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.
That's why I've also started to explicitly decompose constants if possible. Something like `ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours`. Sure, this takes 4 constants and is longer to type than "28 hours".
However, it communicates how I think about this, how these 28 hours come about and how to tweak them if the alerting is being annoying: It's easy to guess that you'd bump that to 29 hours if it throws false positives. But like this, you could see that the backup is supposed to take, say, 3 hours - except now it takes 4 due to growth of the system. So like this, you can now apply an "obviously correct" fix of bumping up the approximate backup duration based on monitoring data.
The comment also leaves me with more questions: how do you handle multiple identical amounts in that window? I would still have to read the implementation to be sure.
I would prefer encoding this in an Uncle Bob style test. It acts as living documentation that cannot get out of sync with the code and explains the why through execution. For example:
This way, the 3 day rule is a hard requirement that fails the build if broken rather than a suggestion in a comment block.Yes, comments can drift out of sync with the code. That doesn't mean the comments were a bad idea; it just means the programmer didn't update the comments when they should have.
Similarly, even with zero comments, variable names can drift out of sync with their intended purpose, e.g. when they are repurposed to hold values that are different from what their name implies. Again, the solution is to rename the variable so it reflects its purpose (or possibly create another variable).
> This way, the 3 day rule is a hard requirement that fails the build if broken rather than a suggestion in a comment block.
What happens when a dev changes the test code, but fails to update the string "should_match_debit_with_credit_only_if_within_three_day_settlement_window"? That's effectively no different than comments being out of sync with the code.
For example, at my last job, we shoved all constants managed by the balancing teams into a static class called BalancingTeam, to make it obvious that these values are not in our control. Tests, if (big-if) written, should revolve around the constants to not be brittle.
However, I’d argue that being 'out of our control' is actually the main reason to test them. We treat these as acceptance tests. The goal isn't flexibility, it is safety. If a PR changes a regulated value, the test failure acts as a tripwire. It forces us to confirm with the PM (and the Jira ticket) that the change is intentional before merging. It catches what code structure alone might miss.
Ultimately disciplined practice requirements rely on disciplined practices to succeed. You can move the place where the diligence needs to taken, but at the end the idea that comments can lose their meaning isn't that different to other non-functional, communicative elements also being subject to neglect. I don't mean to suggest that a longish test title wouldn't be more likely to be maintained, but even with that long name you are losing some context that is better expressed, person-to-person, using sentences or even paragraphs.
I had first hand experiences with this, oddly enough, also working on an accounting system (OK, ERP system... working the accounting bits). I was hired to bring certain deficient/neglected accounting methodologies up to a reasonable standard and implement a specialized inventory tracking capability. But the system was 20 years old and the original people behind the design and development had left the building. I have a pretty strong understanding of accounting and inventory management practices, and ERP norms in general, but there were instances where the what the system was doing didn't make sense, but there was no explanations (i.e. comments) as to why those choices had been taken. The accounting rules written in code were easier to deal with, but when we got to certain record life-cycle decisions, like the life-cycle evolution of credit memo transactions, the domain begins to shift from, "what does this mean from an accounting perspective", where generally accepted rules are likely to apply, to what did the designers of the application have in mind related to the controls of the application. Sure I could see what the code did and could duplicate it, but I couldn't understand it... not without doing a significant amount of detective work and speculation. The software developers that worked for the company that made this software were in the same boat: they had no idea why certain decisions were taken, if those decisions were right or wrong, or the consequences of changes... nor did they care really (part of the reason I was brought in). Even an out-of-date comment, one that didn't reflect the code has it evolved, would still have provided insight into the original intents. I know as well as you that code comments are often neglected as things change and I don't take it for granted that understanding the comments are sufficient for knowing what a piece of code does.... but understanding the mind of the author or last authors does have value and would have helped multiple times during that project.
When I see these kinds of discussions I'm always reminded of one of my favorite papers. By Peter Naur, "Programming As Theory Building" (https://pages.cs.wisc.edu/~remzi/Naur.pdf). In my mind, comments that were at least good and right at one time can give me a sense of the theory behind the application, even if they cannot tell me exactly how things work today.
edit — also your version forces me to read horizontal rather than vertical, which takes longer ime.
sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now.
For example:
Then it's easier to document what components a constant is composed of using code without introducing unnecessary bindings in the scope of the relevant variable. Sure constants are just data, but the first questions that pops into my head when seeing something in unfamiliar code is "What is the purpose of this?", and the smaller the scope, the faster it can be discarded.I often write things the way you have done it, for the simple reason that, when writing the code, maybe I feel that I might have more than one use for the constant, and I'm used to thinking algebraically.
Except, that I might make them global, at the top of a module. Why? Because they encode assumptions that might be useful to know at a glance.
And I probably wouldn't go back and remove the constants once they were named.
But I also have no problem with unnamed but commented constants like the ones in the comment you responded to.
That's all fine.
Just note that this was one of the easiest examples I could find. For example, for reasons out of my control, the individual network configuration on a linux host is positively nuts. The decision whether to deploy routes, static DNS servers and such depends on 3-5 facts about the datacenter and the provider it's on.
In such a case, it is more maintainable to separate the facts about the provider, or the thing we are looking at (e.g. "Does this provider allow us to configure routes in their DHCP server?", from the computation/decision making ("Can the system rely on the routes from the DHCP servers?"), and all of that from the actual action based off of the decision ("Deploy routes statically if DHCP provided routes are not correct").
The line between the two is not that blurry: assume your reader has total knowledge of programming, and no knowledge whatsoever of the outside world. Comments about what the code does to bits are the "what"; comments about how the code relates to the outside world are the "why". The rest is a matter of taste and judgment.
"assume your reader has total knowledge of programming"
Because if I know my fellow programmers have like me not a total knowledge of programming, what comments before footguns seem useful to me.
Or when there was a hack that is not obvious.
To me it mostly is not a question of taste, but context. Who will read the code?
In my experience, when I review junior contributors’ code and see “what” comments, it’s usually caused by 1) bad naming, or 2) abstractions that don’t make sense, or 3) someone trying to reinvent maths but incorrectly, or 4) missing tests, or 5) issues with requirement gathering / problem specification, or 6) outright laziness where the contributor doesn’t want to take the time to really think things through, or 7) unclear and overcomplicated code, or… any number of similar things.
At the very least, any time you see a “what” comment, it’s valuable to take notice and try really hard to think about whether the problem the comment tries to solve has a better solution. Take it as a personal challenge.
Like something really bad
x=y //triggers method xyz
So I would agree that under controlled conditions, they should not be necessary.
Or to put it another way: to provide the necessary context for figuring out why a particular piece of code is written the way it's done. (Or what it's supposed to do.)
they can also be a good defence against newer seniors on the team refactoring willy nilly — can’t argue with business rules, but can argue over uncle bob. ;)
If the comment was
the comment is more evergreen, until the actual logic changes. If/when the logic changes... update the comment?It seems like the caller only needs to know that it’s checking within the window, which is then likely a constant (like SETTLEMENT_WINDOW_DAYS) that gives the context.
If you need to, you can add a comment to the constant linking to whatever documentation defines why three days.
Sure it does. You've isolated the "what" to the name, and the comments can focus on the "why".
(Although I think some of the XP advocates would go off the rails and try to model a settlement window as an object....)
Gives a good idea of what but not why and if anyone considers that method name too long they can get back in their cave, tbh.
When the name is long, it should be more than one function. Finding the transactions within a settlement window seems distinct from matching them up.
The comment about 3 days isn't necessary when "numDays" is a variable in the SettlementWindow class (or other relevant scope). I'd be a lot more comfortable reading code that makes this concept a proper abstraction rather than just seeing a comment about a hard-coded 3 somewhere. Magic values are bad and comments don't demystify anything if they get out of sync with the code.
Yes, there are multiples levels of knowledge, and the required level of commenting depends on the minimum knowledge expected of a reader in each of several dimensions.
In point of fact, the only thing that almost always can do without documentation is a different question, "How."
This is missing from your comment. While I'd probably be able to pick that up from context, I'd have to hold "this is an assumption I'm making" in my head, and test that assumption against all parts of the codebase I encounter until I become certain: this'd slow me down. I'd recommend including the "why" explicitly.
Your comment is a poor example because you unwittingly wrote "why" comments, not "what".
If anything, you proved the importance of clarifying why things must be the way they are.> The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.
You are also completely off in here as well, and I think that your comment shows a deeper misunderstanding of the topic. What you tried to frame as "Uncle Bob approach" actually means implementing your domain in a clear and unambiguous way so that both the "what" and the "why" are clear by looking at the code. Naming conventions are one aspect, but the biggest trait is specifying an interface that forces the caller no option other than doing the what's because of the why's.
Without looking too hard into it, whipping out something like this in a few seconds would match the intent
I don't know how that book ever got as big as it did, all you have to do is try it to know that it's very annoying and does not help readability at all.
https://github.com/johnousterhout/aposd-vs-clean-code
_A Philosophy of Software Design_ is an amazing and under-rated book:
https://www.goodreads.com/en/book/show/39996759-a-philosophy...
and one which I highly recommend and which markedly improved my code --- the other book made me question my boss's competence when it showed up on his desk, but then it was placed under a monitor as a riser which reflected his opinion of it....
At the very least, you convinced me to add John's book to my ever-growing reading list.
I have had so many discussions about that style where I tried to argue it wasn't actually simpler and the other side just pointed at the book.
One of the most infuriating categories of engineers to work with is the one who's always citing books in code review. It's effectively effort amplification as a defense mechanism, now instead of having a discussion with you I have to go read a book first. No thanks.
I do not give a shit that this practice is in a book written by some well respected whoever, if you can't explain why you think it applies here then I'm not going to approve your PR.
IMX, people mainly defend goto in C because of memory management and other forms of resource-acquisition/cleanup problems. But really it comes across to me that they just don't want to pay more function-call overhead (risk the compiler not inlining things). Otherwise you can easily have patterns like:
(I prefer for handling NULL to be the cleanup function's responsibility, as with `free()`.)Maybe sometimes you'd inline the two acquisitions; since all the business logic is elsewhere (in `do_thing_with`), the cleanup stuff is simple enough that you don't really benefit from using `goto` to express it.
In the really interesting cases, `do_thing_with` could be a passed-in function pointer:
And then you only write that pattern once for all the functions that need the resources.Of course, this is a contrived example, but the common uses I've seen do seem to be fairly similar. Yeah, people sometimes don't like this kind of pattern because `cleanup_a` appears twice — so don't go crazy with it. But I really think that `result = 2; goto a_cleanup;` (and introducing that label) is not better than `cleanup_a(a); return 2;`. Only at three or four steps of resource acquisition does that really save any effort, and that's a code smell anyway.
(And, of course, in C++ you get all the nice RAII idioms instead.)
Jumping out of the loop with a goto is also more readable than what Python has to offer. Refactoring things into functions just because you need to control the flow of the program is an anti pattern. Those functions add indirection and might never be reused. Why would you do that even if it was free performance wise?
This is why new low level languages offer alternatives to goto (defer, labelled break/continue, labelled switch/case) that cover most of the use cases.
Imo it's debatable if those are better and more readable than goto. Defer might be. Labelled break probably isn't although it doesn't matter that much.
Python meanwhile offers you adding more indirection, exceptions (wtf?) or flags (inefficient unrolling and additional noise instead of just goto ITEM_FOUND or something).
Then replaced a pretty simple one with an anonymous immediately invoked function that contained a switch statement with a return for each case.
Um, can I have a linter rule against that?
Basically it's a shame that Typescript doesn't have a switch-style construct that is an expression.
And that nowadays you can't make nested ternaries look obvious with formatting because automated formatters (that are great) undo it.
Alas, there's a lot of Go people who enjoy that kind of thing (flashback to when I was looking at an interface calling an interface calling an interface calling an interface through 8 files ... which ended up in basically "set this cipher key" and y'know, it could just have been at the top.)
I said this at a company I worked and got made fun of because "it's so much more organized". My take away is that the average person has zero ability to think critically.
But I definitely don't go about it the same way. Mr. Martin honestly just doesn't seem very good at implementing his ideas and getting the benefits that he anticipates from them. I think the core of this is that he doesn't appreciate how complex it is to create a class, at all, in the first place. (Especially when it doesn't model anything coherent or intuitive. But as Jeffries' Sudoku experience shows, also when it mistakenly models an object from the problem domain that is not especially relevant to the solution domain.)
The bit about parameters is also nonsense; pulling state from an implicit this-object is clearly worse than having it explicitly passed in, and is only pretending to have reduced dependencies. Similarly, in terms of cleanliness, mutating the this-object's state is worse than mutating a parameter, which of course is worse than returning a value. It's the sort of thing that you do as a concession to optimization, in languages (like, not Haskell family) where you pay a steep cost for repeatedly creating similar objects that have a lot of state information in common but can't actually share it.
As for single-line functions, I've found that usually it's better to inline them on a separate line, and name the result. The name for that value is about as... valuable as a function name would be. But there are always exceptions, I feel.
As John Carmack said: "if a lot of operations are supposed to happen in a sequential fashion, their code should follow sequentially" (https://cbarrete.com/carmack.html).
A single method with a few lines is easy to read, like the processor reading a single cache line, while having to jump around between methods is distracting and slow, like the processor having to read various RAM locations.
Depending on the language you can also have very good reasons to have many lines, for example in Java a method can't return multiple primitive values, so if you want to stick to primitives for performances you inline it and use curly braces to limit the scope of its internals.
Things were pretty dire until Sandi Metz introduced Ruby developers to the rest of the programming world with "Practical Object-Oriented Design". I think that helped start a movement away from "clever", "artisanal", and "elegant" and towards more practicality that favors the future programmer.
Does anyone remember debugging Ruby code where lines in stack traces don't exist because the code was dynamically generated at run time to reduce boilerplate? Pepperidge Farm remembers.
It was useful for me and many others, though I never took such (any?) advice literally (even if the author meant it)
Based on other books, discussions, advice and experience, I choose to remember (tell colleagues) it as “long(e.g. multipage) functions are bad”.
I assume CS graduates know better now, because it became common knowledge in the field.
Haskell (and OCaml I suppose two) are outliers though as one is supposed to have a small functions for single case. It's also super easy to find them and haskell-language-server can even suggest which functions you want based on signatures you have.
But in other languages I agree - it's abomination and actually hurt developers with lower working memory (e.g. neuroatypical ones).
But of course understandeable code with comments simply has much more bandwidth of expression so it will get the best of both worlds.
I see writing commentless code like practicing playing piano only with your left hand, it's a showoff and you can get fascinatingly close to the original piece (See Godowsky's Chopin adaptations for the left hand), but of course when you are done showing off, you will play with both hands.
Never heard of "that style of programming" before, and I certainly know that Uncle Bob never adviced people to break down their programs so each line has it's own method/function. Are you perhaps mixing this with someone else?
In his own words, ( page 34 ):
> [functions] should be small. They should be smaller than that. That is not an assertion I can justify.
He then advocates for functions to be 2-3 lines each.
That's both true, but long way away from "every line should have it's own method", but I guess parent exaggerated for effect and I misunderstood them, I took it literally when I shouldn't.
There's a literal link to a literal Uncle Bob post by the literal Uncle Bob from which the code has been taken verbatim.
I have no idea why people want to "save time" to write short comments and short variable names. I just CTRL+C, CTRL+V my variable name anyway. They compound on each other, and that ends up adding an unnecessary level of complexity, and the possibility for errors.
When I come back to a piece of complex code after a year or two, I'm very, very happy that I've used a variable name like "three_tuple_of_weight_radius_price" instead of "tuple" or "t".
Really long variable names put extra space between the operators and calls, which makes it harder to grok the logical structure of the expression. When I write `a + b` in code, it's in that order because the language demands it, and I use languages with that syntax for reasons of familiarity; but really, the `+` is the most important thing there, so it shouldn't be buried way off to the right.
I don't like for variable names to mark type. `weight_radius_price` is fine (and usually much better than `wrp`, and definitely much better than `tuple` or `t`) but a) we know it's a 3-tuple because we can see the right-hand-side of the equals sign; b) the structure of the name already conventionally tells us to expect more or less that (although maybe a class or structure abstraction is missing?); c) the choice of a tuple is probably not really relevant in context as compared to other ways of sticking three atomic values together.
In the context I’m referring to, the variable may have been set 80 lines earlier, 7 years ago, when I was worse at coding.
The point of the redundancy of very clear, descriptive variable names, is that the code is going to suck tomorrow, even if it is good today. Future me is always going to look down on today me. The difference is that I planing for that, so maybe I can help out future me by being verbose, even if it isn’t necessary for much of the code that ends up being fine.
When I have a nasty bug, I don’t want to waste a day on something that could have been clear if I’d just explained it properly.
I do think very long names can hurt readability though so it's still a balancing act, but variables like "t" (often used in Haskell, ugh) is awful in my opinion.
I absolutely going to add the type to the variable name if it’s a complex function. It’s just clearer.
If your weights need to be, say, a floating-point number of kilograms, then you establish that convention, and only mark intentional deviations from the convention (e.g. because you're doing an explicit conversion to format output). (Or you use a library like Pint to give more formality to it.)
If I’m adding a feature to the site in 5 years, it’s going to be important to know if I’ve done that or not.
When I read the replace() method, I was immediately confused because it has no arguments. stringToReplace and alreadyReplaced are properties of the class, so you must look elsewhere (meaning, outside of the function) for their definitions. You also don't know what other bits of the class are doing with those properties when you're not looking. Both of these facts inflate the context you have to carry around in your head.
Why is this a class? In the replace() method, there is a call to translate(symbolName). Why isn't there also a SymbolNameTranslator class with a translate() method? Who decided one was simple enough to use a function while the other warrants a class?
SymbolReplacer surely could also be done with a function. I understand that this is illustration code, so the usage and purpose is not clear (and the original Bob Martin article does not help). Is there a reason we want these half-replaced strings made available every time we call SymbolReplacer.replace()? If there is, we can get the same data by using a reduce function and returning all of the iterations in a list.
A plain, immutable function necessarily contains within it the entire scope of its behavior. It accepts and returns plain data that has no baggage attached to it about the data's purpose. It does one thing only.
I hope not of a lot of the future folks hate me for leaving them with ample context and clear/dead simple code.
In a very real way, a codebase is a conversation among developers. There's every chance the next guy will be less familiar with the code than you, who just did a deep dive and arrived at some critical, poorly documented line of code. There's an even better chance that person will be you, 6 months from now.
Use opportunities to communicate.
There might be a better one that also takes into account whether the code does something weird or unexpected for the reader (like the duplicate clear call from the article).
Names capture ideas. Only if we name something can we (or at least I) reason about it. The more clear and descriptive a name for something is, the less cognitive load is required to include the thing in that reasoning.
TFA's example that "weight" is a better variable name than "w" is because "weight" immediately has a meaning while use of "w" requires me to carry around the cumbersome "w is weight" whenever I see or think about "w".
Function names serve the same purpose as variable names but for operations instead of data.
Of course, with naming, context matters and defining functions adds lines of code which adds complexity. As does defining overly verbose variable names: "the_weight_of_the_red_ball" instead of "weight". So, some balance that takes into account the context is needed and perhaps there is some art in finding that balance.
Comments, then, provide a useful intermediate on a spectrum between function-heavy "Uncle Bob" style and function-less "stream of consciousness" style.
Had to add the last sentence for the circa 2020s developer experience. LLM comments are almost never useful since they're supposed to convey meaningful information to another human coder, anything your human brain can think of will probably be more helpful context.
Bad comments aren’t just less helpful than possible, they’re often harmful.
Once you’ve hit a couple misleading comments in a code base (ie not updated, flatly wrong, or deeply confusing due to term misuse), the calculus swings to actively ignoring those lying lies and reading the code directly. And the kind of mind resorting to prose when struggling to clarify programming frequently struggles with both maintenance and clarity of prose.
If there are no comments, you are reading the code (or some relatively far away document) for all understanding anyway. If there are inaccurate comments, worst case you're in the same boat except maybe proceeding with a bit more caution next comment you come across. I always ask of fellow engineers: why is it unduly difficult to also fix/change the comments as you alter the code they refer to? How and when to use comments is a balancing of trade-offs: potential maintenance burden in the future if next writers are lazy vs. opportunity to share critical context nearest its subject in a place where the reader is most likely to encounter it.
Replace symbol placeholders in the input string with translated values. Scan the string for symbol placeholders that use the format "$foo". The format uses a dollar sign, then optional ASCII letter, then optional word characters. Each recognized symbol is replaced with its corresponding value.
Symbols are only replaced if the symbol exists i.e. getSymbol(String) returns non-null, and the symbol has not already been replaced in this invocation.
Example:
Return the string with symbol placeholders replaced.A single line comment is easy to parse, read and spot as having to be changed when you patch something.
Here is one I wrote just to talk about iterating a loop in reverse:
Or here is another for masking a v4 address, but also explaining why a uint64 is used (this is calculated in a hot loop [same as the previous comment example], so I felt it was imperative to explain what is going on as there is very little room otherwise to optimise):Was it done because shifting by amount equal or greater to integer width is undefined behavior? That would still not require storing result in 64bit mask, just shifting (~0ULL) would be enough. That would be a lot more valuable to explain than how bitwise AND works.
The first one also seems slightly sketchy but without knowing rest of details it's hard to be sure. IPV6 address is 128bits, that's 2 registers worth integers. Calculating base address would take 2 bitwise instruction. Cost of copying them in most cases would be negligible compared to doing the lookup in whatever containers you are searching resulting address. If you are storing it as dynamically allocated byte arrays (which would make copying non trivial) and processing it in such a hot loop where it matters, then seems like you have much bigger problems.
For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps.
As for the 128-bit addresses, we used boost ip::address_v6 to_bytes() as it appears there was no masking option.
For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps.
Ah apologies, too late now, should've mentioned it in the PR. But I expected it would ruffle some feathers, I don't care for conventions or other people's quibbles. As long as it improves understanding for the reader, regardless of how "redundant", then mission accomplished.
I'm trying to think of a good example, maybe something like a pointer window based function (off the top of my head)
(This isn't real code. Don't get hung up on it)
People will quibble, but- I'm not convinced you could change the variable names without harming clarity. Would a name like uniquesEndIndex really be any clearer? It adds noise to the code and still doesn't satisfy a confused reader
- I don't want to use function calls for documentation, eg putInUniques(). I'm doing it this way because I want it to run really quick.
Otherwise I agree, people have a weird hang up about short variable names. Somehow not a problem in mathematics...
I have slightly unorthodox opinions about short variables. I used to hate them. Then I posted a question on one of the PL design forums - it might have been Reddit r/programminglanguages - why is there are history of single letter names for type variables? ie T, U, etc for generics. The answer I got back, was, sometimes you want code to focus on structure rather than identities. That stuck with me, because it helped me understand why so much C code (including Linux) code uses similar naming practices. Names can lie, and sometimes expressing the structure is the absolute critical thing.
The specificity or abstractness of a (variable) name relates to the values that it can hold. So when you have a very abstract function whose inputs can be of almost any type, naming those inputs in an overly-specific manner is an exact inverse of the failure of giving an overly generic to name highly constrained parameter.
Examples of correct naming:
Examples of incorrect naming: All this said, I do agree with your original take on the comments. I much prefer having human-readable explanations inline with anyhow non-trivial code. If nothing else, they really make it easier to correctly fix the code if a bug is found much later.Those comments also don't really help me quickly understand the code. I'd do a small doc comment along the lines of "Removes repeated elements from the supplied list, returning the remaining items in original order"
Comments should answer the question why you are not using some kind of hash set and do a single pass over the data and why it's OK to reorder the strings. One could reasonable expect that Dedupe shows first occurrences in order.
For example, for some reason padding bytes are allowed in variable length integers (LEB128) for reasons I still do not understand:
You might wonder how this is different than the example you provided, and the answer is because you could sneakily intersperse anything you wanted between your imperative bindings (like a conditional return statement), so I actually have to read every line of your code, vs in a lisp `let` I know for a fact there is nothing sneaky going on.
I see this a lot in the wild, though - as an honest question (not trolling!) why do people still shorten their variable names in place of having a terse descriptor ?
If I look through code and see a variable I don't know, I want to see its definition anyway, so I know the type, scope, initial value, etc. And it's trivially possible to do that with everything that has a "jump to definition" command.
While it leads to more things to read, which thus may take time, I feel that the benefits of using comments far outweighs the negative sides. This is even valid when comments are outdated usually. To me, adjusting and updating comments often was much easier and faster than describing something de-novo.
In the ruby land this is quite problematic because many do not use any comments. The result is a horrible code base. Getting people who wrote that code to use comments is often too late, as they already abandoned ruby in favour of another language, so I never buy the cop-out explanation of "the code is self-explanatory" - not even in ruby it is. Everyone can write horrible code.
Of course, be sensible and use good judgement :-)
But, that's what we're paid for, right?
Change my mind!
The sane thing would be to loop through building the output string, adding the replacement for each symbol as you go. Using String.replace and the alreadyReplaced map is just a bad idea. Also inefficient, as it and throws away strings and does a redundant search on each loop iteration.
Feels typical of this whole '90s-era culture of arguing over refactoring with Java design patterns and ornate styles without ever thinking about if the algorithm is any good.
Edit: also, consider $foo $foobar. It doesn't properly tokenize on replacement, so this will also be wrong.
As follows:
(Apparently there's also now a Matcher.replaceAll one could use, but it's arguably cheating to outsource the loop to a method that probably didn't exist when the Uncle Bob version was written, and it's slightly less efficient in the "no such symbol" case.)Coding style must serve the purpose of aiding understanding. If you have strong opinions about the coding style of `replace` but those opinions don't lead to recognition that the implementation was incorrect and inefficient, your opinions are bad and you should feel bad. Stop writing garbage books and blog posts!
</rant>
Thought-provoking article, nonetheless!
He starts implementing any module or function by first writing the documentation for it and let's it guide both the functionality and structure.
Makes Redis also extremely easy and enjoyable to read.
Random example: https://github.com/redis/redis/blob/unstable/src/aof.c
Now I sometimes use this practice when working with agents, if I need something done just a certain way. It's time consuming, but it produces good results.
That comment is not helpful, because it's wrong. The translate() function just is some sort of lookup. It doesn't replace anything. It's the stringToReplace.replace() call that replaces all instances.
I find that ends up being succinct and useful in the future.
Comments should explain. If it’s why, it’s why. If it’s what, it’s what. The point of comments is to explain, to the reader/editor, what the intent of the code is and how you chose to approach it. That’s it. No need to fight over what, why, how, who, etc.
If you have domain knowledge that helps support the code, maybe a link to further reading, by all means add it to comments please!
There have been so many times where I have commented _why_ I think some uncle bob-ism made the code unclear and the response is always:
> *Sends link to Clean Code, maybe you don't know about this?
No, I do, and I am allowed to disagree with it. To which they always clutch their pearls "How do you think you know better than uncle bob!?", this is a Well Established Pattern TM.
I don't think I know better than Uncle Bob, but I don't think Uncle Bob works on this codebase nearly as much as you or I.
If that needs a why it’s a why-comment.
If it needs a what it’s a what-comment. Especially if clever programming tricks are used. 6 month later you already forgot what the trick is and how it works.
The "why" is the part of the explanation that can't be deduced from the code.
https://en.wikipedia.org/wiki/Fast_inverse_square_root
That definitely needs a what comment.
But there's also the case to be made that the comments that particular code needs are "why" comments. I can see what happens, but why does it work?
The article mentions the reason for the code which I would expect in the commit message
- clean code one, for me it just reads easier, specific bits of the larger operation are not surrounded with noise coming from other bits like it is in the commenting "what" one. I can focus more on each step, and that can make it easier to spot a bug or to refactor/adjust/extend, as now I'm more like on a given page of Lego set instructions
- the commenting of "what" one - yeah, this obviously makes it quicker (but probably not easier) to see the larger picture. It has benefits, but I believe this helps more hacker/scripter kind of people than programmers
It makes it easier for dev's brain to parse the code e.g. to understand what code really does, while fattier but commented version makes it harder but tries to replace it with information about original coder's intentions. Which is maybe important too but not as important as code itself.
Not to forget that it's too easy to change code and leave comments obsolete.
> Technically this performs worse because you lose short-circuiting
Not really, because optimizing compilers are a thing, when this thing is parsed into SSA, there won't be a difference.
I just tested a recent gcc at -O2 with a contrived example using strings in an unordered_set: a look-up always occurs if not relying on short-circuiting
Even in stupidest cases, like:
The rationale for this is that if you start to mute developer on case of "this is not comment-worthy" you start to actually losing context for the codebase. Sure, maybe "add two and two" is not contextful enough, but "add same account twice" might be signal for appropriate code.Maybe that's me, but I rarely saw teams which over-document, under-documenting is usually the case. So if we ever meet in professional environment - comment away. Worst case scenario - I'll skim over it.
This is a good point, although this recently changed with LLMs, which often spit out a ton of redundant comments by default.
I used to comment in a similar way to claude/chatgpt, as in both the what and why. (although the why in LLMs is often lost. ) I used to comment as if it was for someone who didn't know the libraries very well. (ie me in two years time. Documentation at the previous place was utterly shite, so it was effectively a journal of discovery)
However, my commenting style is both what and why. My variable names can be longer than my FANG peers, mainly because I know that English not being a first language means that half arsed abbreviations are difficult to parse.
But effort is placed on not using fancy features, python mostly, so avoiding lambdas and other "syntactic sugar", unless they absolutely make sense. If I do use them, i have a comment saying _why_ I'm doing it.
Some of my colleagues were dead against any kind of commenting. "my code should be readable enough without it". They had the luxury of working on one bit for a few months, then throwing it away.
Only comments should explain what, variable names should only hint
The first example is perfectly fine, nobody has the time to derive or read a verbose formula involving the words 'weight', 'radius' and 'price'.
But I don’t think they’re worth it. My issue with “what” comments is they’re brittle and can easily go out of sync with the code they’re describing. There’s no automation like type checking or unit tests that can enforce that comments stay accurate to the code they describe. Maybe LLMs can check this but in my experience they miss a lot of things.
When “what” comments go out of sync with the code, they spread misinformation and confusion. This is worse than no comments at all so I don’t think they’re worth it.
“Why” comments tend to be more stable and orthogonal to the implementation.
Same goes for comments vs commit messages. It's a fact comments get outdated and then you have an even bigger problem whereas a commit message is always correct at the time it was made. But obviously again, no hard rules. Sometimes I feel it's better to write a comment, e.g. if it's something that is really important and won't change a lot.
In my early days I read a lot of "you should" "this is wrong". But code is an expressive medium, you can do things one way, or do it the other, you can write "amountOfEmployees" or you can write "ne" with an "#amount of employees" comment, either way is absolutely fine and you can use whichever depending on your priorities, tradeoffs or even your mood.
Also I used to obsess over code, (and there's a lot of material that obsesses about code), but after you become profficient at it, there's a cap on the returns of investing time into your codebase, and you start to focus on the product itself. There's not much difference between a good codebase and a marvelously polished codebase, so you might as well use the extra focuse on going from a bad UX to a neutral UX or whatever improvement you can make to the product.
"When" is occasionally a good question to answer in a comment, e.g. for an interrupt handler, and "Where" is also occasionally a good thing to answer, e.g. "this code is only executed on ARM systems."
The other three questions typically form a hierarchy: Why -> What -> How.
A simplistic google shows that "code comments" are next to "what" and "how" at about the same frequency as they are next to "why" and "what."
This makes some amount of sense, when you consider the usual context. "Why" is often an (assumed) obvious unstated business reason, "What" is a thing done in support of that reason, and "How" is the mechanics of doing the thing.
But with multiple levels of abstraction, _maybe_ the "What" inside the hierarchy remains a "What" to the level above it, but becomes a "Why" to the next level of "What" in the hierarchy. Or maybe the "How" at the end of the hierarchy remains a "How" to the level above it but becomes a "What" to a new "How" level below it.
Is it:
Why -> What/Why -> What/Why -> What/Why -> What -> How
or
Why -> What -> How/What -> How/What -> How/What -> How
In many cases the intermediate nodes in this graph could be legitimately viewed as either What/Why or as How/What, depending on your viewpoint, which could partly depend on which code you read first.
In any case, there are a few hierarchies with final "Hows" that absolutely beg for comments (Carmack's famous inverse square root comes to mind) but in most problem domains that don't involve knowledge across system boundaries (e.g. cache optimization, atomic operations, etc.), the final "How" is almost always adequately explained by the code, _if_ the reader understands the immediately preceding "What."
If I see a function "BackupDatabase()" then I'm pretty sure I already know both "Why" and "What" at the highest levels. "How" I backup the database might be obvious once I am reading inside the function, or the code might be opaque enough that a particular set of lines requires explanation. You could view that explanation as part of "How" the database is backed up, or you could view that explanation as "What" the next few lines of code are doing.
Again, this viewpoint might even partly depend on where you started. If you are dumped inside the function by a debugger, your question might be "What the heck is this code doing here?" but if you are reading the code semi-linearly in an editor, you might wonder "How is BackupDatabase() implemented?"
If you have the spare time, you can try and submit your patches upstream; in the meantime, you just maintain your own version.