ccloveak/intermediate-node-course

Refactor

Closed this issue · 1 comments

At this point, your server.js file should look something like this:

const express= require('express');
const mongoose= require('mongoose');
const bodyParser= require('body-parser');
const port=8000;
const app= express();

const User=require('./models/User');
mongoose.connect('mongodb://localhost/userData')

app.use(bodyParser.json());

app.listen(port, ()=>{
  console.log(`server is listening on port:${port}`)
})

app.post('/users',(req,res)=>{
  User.create(
    {
      name:req.body.newData.name,
      email:req.body.newData.email,
      password:req.body.newData.password
    },
    (err,data)=>{
      if (err){
        res.json({
          success: false,
          message: err
        })
      } else if (!data){
        res.json({
          success: false,
          message: "Not Found"
        })
      } else {
        res.json({
          success: true,
          data: data
        })
      }
    }
  )
})

app.route('/users/:id')
.get((req,res)=>{
  User.findById(
    req.params.id,
    (err,data)=>{
      if (err){
        res.json({
          success: false,
          message: err
        })
      } else if (!data){
        res.json({
          success: false,
          message: "Not Found"
        })
      } else {
        res.json({
          success: true,
          data: data
        })
      }
    })
})
.put((req,res)=>{
  User.findByIdAndUpdate(
    req.params.id,
    { 
      name:req.body.newData.name,
      email:req.body.newData.email,
      password:req.body.newData.password
    },
    {
      new:true
    },
    (err,data)=>{
      if (err){
        res.json({
          success: false,
          message: err
        })
      } else if (!data){
        res.json({
          success: false,
          message: "Not Found"
        })
      } else {
        res.json({
          success: true,
          data: data
        })
      }
    })
})
.delete((req,res)=>{
  User.findByIdAndDelete(
    req.params.id,
    (err,data)=>{
      if (err){
        res.json({
          success: false,
          message: err
        })
      } else if (!data){
        res.json({
          success: false,
          message: "Not Found"
        })
      } else {
        res.json({
          success: true,
          data: data
        })
      }
    })
})

This code works, but it is pretty repetitive. In general, it is a good practice to refactor code like this so it is easier to maintain.

Create function for response

We are handling the response in the same way each time. We should define a function to shorten this and make it easier to maintain. Paste this in your server file above your routes:

function sendResponse(res,err,data){
  if (err){
    res.json({
      success: false,
      message: err
    })
  } else if (!data){
    res.json({
      success: false,
      message: "Not Found"
    })
  } else {
    res.json({
      success: true,
      data: data
    })
  }
}

Now, let's call the "sendResponse" function in our mongoose callback functions:

app.post('/users',(req,res)=>{
  User.create(
    {
      name:req.body.newData.name,
      email:req.body.newData.email,
      password:req.body.newData.password
    },
    (err,data)=>{sendResponse(res,err,data)}
  )
})

app.route('/users/:id')
.get((req,res)=>{
  User.findById(
    req.params.id,
    (err,data)=>{sendResponse(res,err,data)})
})
.put((req,res)=>{
  User.findByIdAndUpdate(
    req.params.id,
    { 
      name:req.body.newData.name,
      email:req.body.newData.email,
      password:req.body.newData.password
    },
    {new:true},
    (err,data)=>{sendResponse(res,err,data)})
})
.delete((req,res)=>{
  User.findByIdAndDelete(
    req.params.id,
    (err,data)=>{sendResponse(res,err,data)})
})

You can always replace the generic sendResponse function with a custom callback, if you need more flexibility.

Using spread syntax (...)

We can shorten these routes further by using the JavaScript spread syntax, which copies each key/value pair in our newData object.

So, rather than this:

{
  name:req.body.newData.name,
  email:req.body.newData.email,
  password:req.body.newData.password
}

we can write this:{...req.body.newData}.

Our routes should look like this now:

app.post('/users',(req,res)=>{
  User.create(
    {...req.body.newData},
    (err,data)=>{sendResponse(res,err,data)}
  )
})

app.route('/users/:id')
.get((req,res)=>{
  User.findById(
    req.params.id,
    (err,data)=>{sendResponse(res,err,data)})
})
.put((req,res)=>{
  User.findByIdAndUpdate(
    req.params.id,
    {...req.body.newData},
    {new:true},
    (err,data)=>{sendResponse(res,err,data)})
})
.delete((req,res)=>{
  User.findByIdAndDelete(
    req.params.id,
    (err,data)=>{sendResponse(res,err,data)})
})

The code above is much easier to maintain. Also, with the spread syntax you only need to include values that are being changed. In the first version we were explicitly looking for a value at each key to update. If the values were not included, they would be set to "null". Now our routes are more flexible, since the object to update is defined by the request. This might seem weird at first, but the model should keep your data consistent.

Test this in Postman to make sure you refactored correctly. When you are finished, commit these changes and push to GitHub.

Good Job! 👍 You are finished! But, there is still much more you can do with Node.js, Express.js, and MongoDB. If you want to continue working on this, here are some next steps to improve security:

  1. Encrypt your user passwords with a library like "bcrypt" before saving them.

  2. Add a nested collection (activities, pets, blog posts, etc... ) to your User model using the "populate" method.

  3. Use a library like JSON web token to authenticate your users in a login route.

  4. Use middleware for your mongoose model or express routes to check if there is a valid token that matches the user's id before saving data.