marwinburesch / marwinburesch/intermediate-node-course
Refactor
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 0
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
At this point, your server.js file should look something like this:
```javascript
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:
```javascript
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:
```javascript
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](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax), which copies each key/value pair in our newData object.
So, rather than this:
```javascript
{
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:
```javascript
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*.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with server.js and compare the existing routes with the refactored examples in the issue. Check the shared response handling and request update data, then use Postman against the user endpoints to confirm the refactored routes preserve their behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- express, javascript, node.js, postman
- Domain
- api, backend, databases, testing
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100