Live data from Hacker News

Self-Documenting Code

lackofimagination.org

61–70 of 131 posts

Re: Self-Documenting Code

#61
post #4

I've been developing for a very long time and I'm neither on the side of "lots of comments" or "all code should speak for itself". My philosophy is that comments should be used for two things: 1) to explain code that is not obvious at first glance, and 2) to explain the rationale or humanitarian reasons behind a bit of code that is understandable, but the reasons for its existence are unclear. No philosophy is perfec…

Argh! While I agree 100% with your thoughts here I find how you wrote the list of rules to be very hard to read. And I don't like the comment in the middle of the rules, move it above the declaration. Regex is about the opposite of self documenting as it comes, anything non-trivial needs an explanation!

Well yeah, that's why I said it was "art" :). It's very subjective what's readable and what isn't, and putting code on HN without syntax highlighting (and with the ironic difficulty of editing code here) is one of hardest ways to communicate.

Re: Self-Documenting Code

#62

Earlier quoted context omitted.

After that step, they say "The resulting code is shorter and has no nested logic." The resulting code has the same logic as before, it's just not visually represented as being nested. I've seen the same argument ("nesting is bad so indentation is a code smell") used to say that it's better to use early returns and omit the `else` block, eg: if (some_condition) { // do stuff here return; } // do other stuff here is "b…

I would say (as all good technical people know) it depends. I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later. It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to t…

I call them guard conditions and it’s my preferred pattern too. Deal with all the exceptional cases at the top and then move on to the happy path as a linear flow.

Re: Self-Documenting Code

#63
post #62

Earlier quoted context omitted.

I would say (as all good technical people know) it depends. I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later. It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to t…

I call them guard conditions and it’s my preferred pattern too. Deal with all the exceptional cases at the top and then move on to the happy path as a linear flow.

And at the start of the happy path, I tend to put "Do it" as a comment (a convention I learned from reading Emacs Lisp code).

Re: Self-Documenting Code

#64
post #53

Earlier quoted context omitted.

I don't know. I really don't see any clarity improvements between, `user.password.length >= 8 && passwordRules.every((rule) => rule.test(user.password))` and `validatePassword(password)`. What if you want to add that the password must contain one special character? You don't actually know, by reading the name "validatePassword", if that work has already been done or not. You need to go read the function definition an…

`validatePassword` is definitely clearer. When I'm getting familiar with a new codebase, I don't need to know how each thing happens, I need to know what happens. In fact, in your example, you've already abstracted testing against the rules. (AAMOF, the password length requirement should be another rule...) Also with that example, that is about the limit I would tolerate in a conditional that decides the next step in…

It is definitely not clearer to me. I have no idea what happens in the `validatePassword` function. Does it make a synchronous network call that will stop the world for half a second? Does it throw an exception, or return an error object? I will also have to search the rest of the code to see who else calls it, and potentially refactor those callers as well.

Any smaller function broken out of a larger function is (slightly) confusing. I find that a genuinely large function is much more confusing than two functions half the size, but a petite little function less than 20 lines long is not confusing to begin with.

Re: Self-Documenting Code

#65
post #56

I lived in the C# world for a while and our style guides mandated that we use those JSDoc style comments for every function definition. I loathed them. They invariable became a more verbose and completely redundant version of the function definition. Developers even used a tool (GhostDoc, IIRC) to generate these comments so that CreateNewUser() became // Create New User. Nobody ever read them, few ever updated them,…

I'm a library author and maintainer, in C++ rather than C#. I know what you mean about the redundancy, and my solution to that - which, of course, requires management approval in your case - is the following:

1. It is legitimate to skip parts of a doxygen/JSDoc comment (return value, parameter description) if it is _entirely_ trivial. But:

2. You must document the _why_. Well-written function signatures can tell you the "what", but they usually not tell you the "why".

3. You must make an effort to squeeze that non-triviality of a documented aspect. So, you mention corner-case behavior; you relate the use of a parameter to its use elsewhere; and of course - you describe "why"'s: Why that parameter is _not_ of another type, or why it can't be avoided.

I guarantee you, that if you follow rules (2.) and (3.), you will usually not get to apply (1.); and at the same time, your doxygen comments will stop being annoying boilerplate that you automatically gloss over, because you would be much more likely to find useful information there.

Re: Self-Documenting Code

#66

Types are the best form of documentation because they can be used to automatically check for user error, are integral to the code itself, and can provide inline documentation. The more I program in dynamically typed (or even weakly statically typed) languages the more I come to this conclusion.

You're not wrong, but if your code is full of bespoke types that are only relevant to a couple of places where they're used, you hurt interoperability; you lock yourself in to how things are done now; and you may just be shifting the burden of making sense to someplace else in the code.

If you are able to formulate types which can be grasped without reading and re-reading their code - that's a win; and if you are able to express more of your code in terms of more generic non-trivial types and data structures, which are language-idiomatic - then that's a double win, because you're likely to be able to apply library functions directly, write less code, and have your code readers thinking "Oh, variadix is just doing FOO on the BAR structure, I know what that means".

Re: Self-Documenting Code

#67
post #34

My cut: const passwordRules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/]; async function createUser(user) { const isUserValid = validateUserInput(user); const isPasswordValid = user.password.length >= 8 && passwordRules.every((rule) => rule.test(user.password)); if (!isUserValid) { throw new Error(ErrorCodes.USER_VALIDATION_FAILED); } if (!isPasswordValid) { throw new Error(ErrorCodes.INVALID_PASSWORD); } cons…

“ Don't use a bunch of tiny functions. This makes it harder for future eng to read the code …” This is where the naming things bit comes in. You name the function correctly, then when the body is read to understand that it works as named, you can remove that cognitive complexity from your brain and continue on. Once you’ve built trust in the codebase that things do what they claim, you can start getting a top-down vi…

Abstraction is way better, I don't really want to know how the password is validated unless I know I'm facing issues with validation (which proper logging tells you about before you even dive into the code).

I don't understand why some people prefer being swarmed with details. It's not that they want details, but that they just hate navigating files (layer 8 + tooling problem) or that they "need" to know the details because not knowing them haunts them at night somehow. Also, not having that as a free function makes me think it's not tested at all (although there might be some integration test that hopefully catch all errors at once, but I'm sure they don't either)

Re: Self-Documenting Code

#68
post #34

My cut: const passwordRules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/]; async function createUser(user) { const isUserValid = validateUserInput(user); const isPasswordValid = user.password.length >= 8 && passwordRules.every((rule) => rule.test(user.password)); if (!isUserValid) { throw new Error(ErrorCodes.USER_VALIDATION_FAILED); } if (!isPasswordValid) { throw new Error(ErrorCodes.INVALID_PASSWORD); } cons…

>1

This is one of my pet peeves. I had an engineer recently wrap Dapper up in a bunch of functions. Like, the whole point of dapper to me is that it gets out of your way and lets you write very simple, declarative SQL database interactions and mapping. When you start wrapping it up in a bunch of function calls, it becomes opaque.

DRY has been taken as far too much gospel.

Re: Self-Documenting Code

#69
post #16

Typescript looks much, much better than what he ends up with. The typescript is more or less the same thing but with comment tokens removed. How is just removing the comment tokens not an obvious improvement in readability? Honestly, I think all of jsdoc, pydoc, javadoc, doxygen is stuff that most code should not use. The only code that should use these is code for libraries and for functions that are used by hundred…

[deleted]

Re: Self-Documenting Code

#70
post #34

My cut: const passwordRules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/]; async function createUser(user) { const isUserValid = validateUserInput(user); const isPasswordValid = user.password.length >= 8 && passwordRules.every((rule) => rule.test(user.password)); if (!isUserValid) { throw new Error(ErrorCodes.USER_VALIDATION_FAILED); } if (!isPasswordValid) { throw new Error(ErrorCodes.INVALID_PASSWORD); } cons…

Yep, I’d hire you, and not OOP. There was some really questionable code structuring in there and I always wince at abstraction for abstraction’s sake. It always becomes a hindrance when something goes wrong, which is exactly when you don’t want to have a hard time deciphering code.
Post reply on HN