Comment by rglover
5 years ago
FWIW the clean code approach led me to this pattern which has allowed me to build some seriously complex systems in JS/Node: https://ponyfoo.com/articles/action-pattern-clean-obvious-te....
I agree with the sentiment that you don't want to over abstract, but Bob doesn't suggest that (as far as I know). He suggests extract till you drop, meaning simplify your functions down to doing one thing and one thing only and then compose them together.
Hands down, one of the best bits I learned from Bob was the "your code should read like well-written prose." That has enabled me to write some seriously easy to maintain code.
> your code should read like well-written prose
That strikes me as being too vague to be of practical use. I suspect the worst programmers can convince themselves their code is akin to poetry, as bad programmers are almost by definition unable to tell the difference. (Thinking back to when I was learning programming, I'm sure that was true of me.) To be valuable, advice needs to be specific.
If you see a pattern of a junior developer committing unacceptably poor quality code, I doubt it would be productive to tell them Try to make it read more like prose. Instead you'd give more concrete advice, such as choosing good variable names, or the SOLID principles, or judicious use of comments, or sensible indentation.
Perhaps I'm missing something though. In what way was the code should read like well-written prose advice helpful to you?
Specifically in relation to naming. I was caught up in the dogma of "things need to be short" (e.g., using silly variable names like getConf instead of getWebpackConfig). The difference is subtle, but that combined with reading my code aloud to see if it reads like a sentence ("prose") is helpful.
For example, using what I learned I read this code (https://github.com/cheatcode/nodejs-server-boilerplate/blob/...) as:
"This module is going to generate a password reset token. First, it's going to make sure we have an emailAddress as an input, then it's going to generate a random string which I'll refer to as token, and then I want to set that token on the user with this email address."
So you're interpreting it to mean use identifiers which are as descriptive as they can practically be, and which are meaningful and self-explanatory when used in combination.
I agree that's generally good advice; the mathematical style of using extremely short identifiers generally just confuses matters. (Exception: very short-lived variables whose purpose is immediately clear from the context.) It's only one possible interpretation of code should read like prose, though. It you who deserves the credit there, not Bob.
> silly variable names like getConf instead of getWebpackConfig
To nitpick this particular example: I'd say that, if it's a method of an object which is itself specific to WebPack, then the shorter identifier is fine.
I'm in the "code should read like poetry" camp. Poetry is the act of conveying meaning that isn't completely semantic - meter and rhyme being the primary examples. In code, that can mean maintaining a cadence of variable names, use of whitespace that helps illuminate structure, or writing blocks or classes where the appearance of the code itself has some mapping to what it does. You can kludge a solution together, or craft a context in which the suchness of what you are trying to convey becomes clear in a narrative climax.
> use of whitespace that helps illuminate structure,
Good luck with that now that everybody uses automatic linters & formaters that mess up your whitespace because of some stupid rule that there should be only one empty line and no spaces after a function name or something.
I like to think of the way Hemmingway writes.
Code should be simple and tight and small. It should also, however, strive for an eighth grade reading level.
You shouldn't try to make your classes so small that you're abusing something like nested ternary operators which are difficult to read. You shouldn't try to break up your concepts so much that while the sentences are easy, the meaning of the whole class becomes muddled. You should stick with concepts everyone knows and not try to invent your own domain specific language in every class.
Less code is always more, right up until it becomes difficult to read, then you've gone too far. On the other hand if you extract a helper method from a method which read fine to begin with, then you've made the code harder to read, not easier, because its now bigger with an extra concept. But if that was a horrible conditional with four clauses which you can express with a "NeedsFrobbing" method and a comment about it, then carry on (generating four methods from that conditional to "simplify" it is usually worse, though, due to the introduction of four concepts that could be often better addressed with just some judicious whitespace to separate them).
And I need to learn how to write in English more like Hemmingway, particularly before I've digested coffee. That last paragraph got away from me a bit.
Absolutely this. Code should tell a story, the functions and objects you use are defined by the context of the story at that level of description. If you have to translate between low-level operations to reconstruct the high level behavior of some unit of work, you are missing some opportunities for useful abstractions.
Coding at scale is about managing complexity. The best code is code you don't have to read because of well named functional boundaries. Natural language is our facility for managing complexity generally. It shouldn't be surprising that the two are mutually beneficial.
I tried to write code with small functions and was dissuaded from doing that at both my teams over the past few years. The reason is that it can be hard to follow the logic if it's spread out among several functions. Jumping back and forth breaks your flow of thought.
I think the best compromise is small summary comments at various points of functions that "hold the entire thought".
The point of isolating abstractions is that you don't have to jump and back and forth. You look at a function, and you understand from its contract and calling convention you immediately know what it does. The specific details aren't relevant for the layer of abstraction you're looking at.
Because of well structured abstractions, thoughtful naming conventions, documentation where required, and extensive testing you trust that the function does what it says. If I'm looking at a function like commitPending(), I simply see writeToDisk() and move on. I'm in the object representation layer, and jumping down into the details of the I/O layers breaks flow by moving to a different level of abstraction. The point is I trust writeToDisk() behaves reasonably and safely, and I don't need to inspect its contents, and definitely don't want to inline its code.
If you find that you frequently need to jump down the tree from sub-routine to sub-routine to understand the high level code, then that's a definite code smell. Most likely something is fundamentally broken in your abstraction model.
> I think the best compromise is small summary comments at various points of functions that "hold the entire thought".
I think this is the best, honestly, it reaps most benefits of small single-use functions, without compromising readability or requiring jumping.
This is also how John Carmarck recommends in this popular essay: http://number-none.com/blow/john_carmack_on_inlined_code.htm...
Check out the try/catch and logging pattern I use in the linked post. I added that specifically so I could identify where errors were ocurring without having to guess.
When I get the error in the console/browser, the path to the error is included for me like "[generatePasswordResetToken.setTokenOnUser] Must pass value to $set to perform an update."
With that, I know exactly where the error is ocurring and can jump straight into debugging it.
Good grief, that pattern looks like it's effectively building a stack trace by hand. Does node not provide stack traces with errors?
2 replies →
Nice! However, none of this is required for this endpoint. Here's why:
1. The connect action could be replaced by doing the connection once on app startup.
2. The validation could be replaced with middleware like express-joi.
3. The stripe/email steps should be asynchronous (ex: simple crons). This way, you create the user and that's it. If Stripe is down, or the email provider is down, you still create the user. If the server restarts while someone calls the endpoint, you don't end up with a user with invalid Stripe config. You just create a user with stripeSetup=false and welcomeEmailSent=false and have some crons that every 5 seconds query for these users and do their work. Also, ensure you query for false and not "not equal to true" here as it's not efficient.
Off topic but is connecting to Mongo on every API hit best practice? I abstract my connection to a module and keep that open for the life of the application.
> "your code should read like well-written prose."
This is a good assertion but ironically it's not Bob Martin's line. He was quoting Grady Booch.
Yes, that one did a lot to me too. Especially when business logic gets complicated, I want to be able to skip parts by roughly reading meaning of the section without seeing details.
One long stream of commands is ok to read, if you are author or already know what it should do. But otherwise it forces you to read too many irrelevant details on a way toward what you need.