this post was submitted on 21 Oct 2024
83 points (86.1% liked)

Programming

17248 readers
300 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities [email protected]



founded 1 year ago
MODERATORS
you are viewing a single comment's thread
view the rest of the comments
[–] Kissaki 1 points 3 hours ago* (last edited 3 hours ago)

Code before:

async function createUser(user) {
    if (!validateUserInput(user)) {
        throw new Error('u105');
    }

    const rules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/];
    if (user.password.length >= 8 && rules.every((rule) => rule.test(user.password))) {
        if (await userService.getUserByEmail(user.email)) {
            throw new Error('u212');
        }
    } else {
        throw new Error('u201');
    }

    user.password = await hashPassword(user.password);
    return userService.create(user);
}

Here's how I would refac it for my personal readability. I would certainly introduce class types for some concern structuring and not dangling functions, but that'd be the next step and I'm also not too familiar with TypeScript differences to JavaScript.

const passwordRules = [/[a-z]{1,}/, /[A-Z]{1,}/, /[0-9]{1,}/, /\W{1,}/]
function validatePassword(plainPassword) => plainPassword.length >= 8 && passwordRules.every((rule) => rule.test(plainPassword))
async function userExists(email) => await userService.getUserByEmail(user.email)

async function createUser(user) {
    // What is validateUserInput? Why does it not validate the password?
    if (!validateUserInput(user)) throw new Error('u105')
    // Why do we check for password before email? I would expect the other way around.
    if (!validatePassword(user.password)) throw new Error('u201')
    if (!userExists(user.email)) throw new Error('u212')

    const hashedPassword = await hashPassword(user.password)
    return userService.create({ email: user.email, hashedPassword: hashedPassword });
}

Noteworthy:

  • Contrary to most JS code, [for independent/new code] I use the non-semicolon-ending style following JavaScript Standard Style - see their no semicolons rule with reasoning; I don't actually know whether that's even valid TypeScript, I just fell back into JS
  • I use oneliners for simple check-error-early-returns
  • I commented what was confusing to me
  • I do things like this to fully understand code even if in the end I revert it and whether I implement a fix or not. Committing refacs is also a big part of what I do, but it's not always feasible.
  • I made the different interface to userService.create (a different kind of user object) explicit
  • I named the parameter in validatePassword plainPasswort to make the expectation clear, and in the createUser function more clearly and obviously differentiate between "the passwords"/what password is. (In C# I would use a param label on call validatePassword(plainPassword: user.password) which would make the interface expectation and label transformation from interface to logic clear.

Structurally, it's not that different from the post suggestion. But it doesn't truth-able value interpretation, and it goes a bit further.