Posted by tie-in 4 days ago
Simple is not the same thing as understandable.
They lost me entirely here.
function throwError(error) {
throw new Error(error);
}
async function createUser(user) {
validateUserInput(user) || throwError(err.userValidationFailed);
isPasswordValid(user.password) || throwError(err.invalidPassword);
!(await userService.getUserByEmail(user.email)) || throwError(err.userExists);
What if... [
[() => validateUserInput(user), err.userValidationFailed],
[() => isPasswordValid(user.password), err.invalidPassword],
[() => !(await userService.getUserByEmail(user.email)), err.userExists],
].forEach(function([is_good, error]) {
if (!is_good()) {
throw new Error(error);
}
});
Also on the regex: const rules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/];
No one caught that in all four of these, "{1,}" could be replaced with the much more common "+". A bit odd considering the desire for brevity. I do personally prefer "[0-9]" over "\d", especially considering the other rules, but can go either way on "\W".I might have also added a fifth regex for length though, instead of doing it differently, if my head was in that mode: /.{8,}/
You're doing so much extra work here. Creating many new arrays, running a bunch of extra function calls, creating extra closures, and really obfuscating code from the engine. This will tank performance.
This is the point at which people come back at me with something about "premature optimization" being bad. That's all well and good, but if you prematurely pessimize and let these patterns creep throughout your codebase, you end up with products that are significantly slower than they should be.
I've spent quite a while working on JS engines, and it always impresses me how much extra work exists in JS developers' code, seemingly for no real reason, and it's slowing down the entire internet. This doesn't appear to be better for the developer, the user, or any potential future maintainers.
function isPasswordValid(password) { const issues = []; if (password.length < 8) issues.push("Minimum length is 8 characters"); if (!/[a-z]/.test(password)) issues.push("Must contain at least one lowercase letter"); if (!/[A-Z]/.test(password)) issues.push("Must contain at least one uppercase letter"); if (!/[0-9]/.test(password)) issues.push("Must contain at least one digit"); if (!/\W/.test(password)) issues.push("Must contain at least one special character"); return issues.length > 0 ? issues : ["Password is valid"]; }
> I’m not a fan of TypeScript, but I appreciate its ability to perform static type checks. Fortunately, there’s a way to add static type checking to JavaScript using only JSDoc comments.
If you're writing JSDoc comments, then you're not writing what the author considers to be "self-documenting code."
I wish the author had explained why they are not a fan of TypeScript. Compile time type-safety aside, as the author acknowledges by implication adding type specificity negates the usefulness of JSDoc comments for this particular situation.
I'm personally a big proponent of "self documenting code" but I usually word it as "code that serves as its own documentation because it reads clearly."
Beyond "I would personally use TypeScript to solve that problem", my case for why ALL comments are a code smell (including JSDoc comments, and in my personal opinion) is:
- They are part of your code, and so they need to be maintained just like the rest of your code
- But ... they are "psychologically invisible" to the majority of developers. Our IDEs tend to gray them out by default etc. No one reads them.
- Therefore, comments can become out of sync with the code quite easily.
- Comments are often used to explain what confusing code does. Which means that instead of fixing the code to add clarity, they do nothing but shine a spotlight on the fact that the code is confusing.
- In doing the above, they make messy code even messier.
I am slightly amenable to the idea that a good comment is one that explains WHY weird code is weird. Even then, if you have the luxury of writing greenfield code, and you still need to do something un-intuitive or weird for really good reasons ... you can still write code that explains the "why" through good naming and separation of concerns.
The only time that I would concede that a code comment was the best way to go about things in context is when you're working with a very large, legacy commercial code-base that is plagued by existing tech debt and you have no good options other than to do your weird thing inline and explain why for logistical and business reasons. Maybe the refactor would be way too risky and the system is not under test, the business has its objectives and there's just no way that you can reasonably refactor in time etc. This happens... but professional developers should ideally treat incremental refactoring as a routine part of the development lifecycle so that this situation is as unlikely as possible to arise in the future.
I don't think your code base needs to be very large, or very legacy in order for comments to be valuable or even the best way forward. If the decision exists between a somewhat large refactor or a one-off comment to account for an edge case, I'm likely to take the latter approach every time. Refactors introduce risk, add time, and can easily introduce accidental complexity (ie: an overengineered solution). Now once that edge case becomes more common, or if you find yourself adding different permutations, yeah I agree that an incremental refactor is probably warranted.
That said, perhaps that comment could — and certainly one should at least supplement it — be replaced with a unit test, but I don't think its presence harms anything.