CODEWITHSUNDEEP
javascriptnode.jsexpress
tiomno
tiomno

Reputation: 2228

Reusing code in Node.js module

I just want to know if you'd do a refactoring to reuse code the same way I did here. Sorry for the following long context. :)

I'm learning Express.js with a simple web app and working on the login page with a reset password form. This form asks for an email, which is checked against the DB and then a token and an expiration time of 1 hour is set in the user profile and a URL is sent to the user. The URL is something like http://mywebsite.com/account/reset/43aea78ba678fd8ed746b2b0b79c34da9380a5a6 so when the user accesses this URL I have a couple of routers to deal with the password reset from here:

Here is the module (controller) which has the logic to deal with these tasks:

const mongoose = require( 'mongoose' )
const User = mongoose.model( 'User' )
const promisify = require( 'es6-promisify' )

const findUserByTokenAndDate = ( token, date ) => {
    return User.findOne( {
        resetPasswordToken: token,
        resetPasswordExpires: { $gt: date },
    } )
}

exports.reset = async ( req, res ) => {
    // const user = await User.findOne( {
    //     resetPasswordToken: req.params.token,
    //     resetPasswordExpires: { $gt: Date.now() },
    // } )
    const user = await findUserByTokenAndDate( req.params.token, Date.now() )

    if ( ! user )
    {
        req.flash( 'error', 'Password reset token is invalid or has expired' )
        return res.redirect( '/login' )
    }

    res.render( 'reset', { title: 'Reset your Password' } )
}

exports.confirmedPasswords = ( req, res, next ) => {
    if ( req.body.password === req.body['password-confirm'] )
    {
        return next()
    }

    req.flash( 'error', 'Passwords do not match!' )
    res.redirect( 'back' )
}

exports.update = async ( req, res ) => {
    // const user = await User.findOne( {
    //     resetPasswordToken: req.params.token,
    //     resetPasswordExpires: { $gt: Date.now() },
    // } )
    const user = await findUserByTokenAndDate( req.params.token, Date.now() )

    if ( ! user )
    {
        req.flash( 'error', 'Password reset is invalid or has expired' )
        return res.redirect( '/login' )
    }

    const setPassword = promisify( user.setPassword, user )
    await setPassword( req.body.password )

    user.resetPasswordToken = undefined
    user.resetPasswordExpires = undefined
    const updateUser = await user.save()
    await req.login( updateUser ) // This is to tell password.js which user to log in

    req.flash( 'success', 'Your password has been reset! You are now logged in' )
    res.redirect( '/' )
}

The commented code is the one I reused with the function findUserByTokenAndDate.

Please, note that this is a very simple piece of code that might not even worth to reuse, but I'm looking for good practices in the case of a more complex or larger piece of code.

Thanks!

Upvotes: 1

Views: 387

Answers (1)

Cisco
Cisco

Reputation: 22952

Is this more testable than other solutions?

Depends on who you ask. Yes for me since you're reusing the same logic in other places, it makes sense to abstract it out to it's own function. However, if it's only being used in two places then you don't necessarily have/need to extract it to avoid duplicates. It saves time being able to see the logic of the code right there rather than having to track down the module it's in.

Would you have created a new module, just for keeping code like the one in the function findUserByTokenAndDate?

I would have created a separate module with any/all utility functions such as findUserByTokenAndDate. Then I can just test just the utility functions and nothing else.

Is this good practice?

Some would say that it's over-engineering.

Upvotes: 1

Related Questions