Self-Documenting Code
51–60 of 131 posts
Re: Self-Documenting Code
#52Earlier quoted context omitted.
just because a footgun exists, doesn't mean you should use it
Then let's talk—with specifics!—about why it's a footgun and we shouldn't use it. "Because Go doesn't support it" would also be a reason to avoid: * Generics (at least until recently) * Classes * Prototypes * Exceptions * Async/Await * Package managers (until recently) * Algebraic data types * Effect types * Type inference * Type classes * Etc. You could argue that one or more of these are footguns, but I seriously d…
Re: Self-Documenting Code
#53My 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…
Once a function gets to a certain threshold of size and reuse, I tend to agree with you. But I think that threshold is quite large - like at least 40-50 lines, or reused at least 3 times.
Re: Self-Documenting Code
#54My 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…
My issue with this is that you're using exceptions for control flow. A user not being valid is expected (duplicate username). A password not matching a regex is also expected. Then, in general (not seen here as there are no types), I like to use a lot of types in my code. The incoming user would be of type UnvalidatedUser, whereas the return type of this function would be StoredUser or something like that to distingu…
Re: Self-Documenting Code
#55Earlier quoted context omitted.
“ 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…
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…
Also with that example, that is about the limit I would tolerate in a conditional that decides the next step in this app. I just need to know "if the password is valid, go to $SCREEN; if not, turn on these messages." I don't care about a string of expressions, I care about what it's doing and I don't want to parse the code every time to figure out "oh, this is just password validation."
This is different from verifying that features are implemented - now I need to know the minutia.
Re: Self-Documenting Code
#56Re: Self-Documenting Code
#57I'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…
In fact, I would argue its not only part of it, it's an essential part. I've worked on projects that were written by folks who were very into the "the source code is the documentation" and comments were extremely minimal. Which was fine for the most part, they did a pretty good job at naming things. The problem was what happened when you were trying to debug a problem. The problems usually start looking like this:
"I see ${function} code does X, and that's causing Y which ultimately leads to bug Z, is ${function} supposed to do X, and we should be checking the condition at another level where Y happens or should ${function} not be doing X and that's the root bug?". And no one knows the answer because there's no documentation about what the function is SUPPOSED to do. I view actual comments and documentation about code as the "parity bit" equivalent for CRC checks / RAID 5 storage. If the code and the documentation agree, great now we can have a discussion about whether the code needs to change. If the code and the documentation disagree, then the code is wrong just like if I get data and a parity that don't match, the data is wrong. Your example of the "AT_LEAST_ONE_SYMBOL" with the associated comment is a perfect example of why this is critical. If some months down the line spaces in the password is causing a problem, I now know the code is supposed to be allowing spaces and we know we need to fix the part that's choking on spaces, not the validation rule.
Yes it's possible the documentation/parity is wrong, but in either case it calls out a large warning that we're out of sync at this point, and need to get in sync before we can continue.
Re: Self-Documenting Code
#58Earlier quoted context omitted.
“ 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…
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…
The exceptions to this are situations where you need to do a bunch of sequential steps and cases where you are looping on stuff in higher dimensions.
I have gotten to the point that I consider a comment about the code to be a sign of trouble. (Comments about what the code is interacting with are a different matter.)
Re: Self-Documenting Code
#59I don't find this easier to read: !(await userService.getUserByEmail(user.email)) || throwError(err.userExists); I guess if I worked in a codebase that used that pattern consistently I'd get used to it pretty quickly, but if I dropped into a new codebase that I didn't work on often I'd take a little bit longer to figure out what was going on.
Re: Self-Documenting Code
#60I'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…
Regex is about the opposite of self documenting as it comes, anything non-trivial needs an explanation!