NW6 | Fikret Ellek| Module Servers | Chat server API project | Week 2

[fikretellek]

Learners, PR Template

Self checklist

• I have committed my files one by one, on purpose, and for a reason
• I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
• I have tested my changes
• My changes follow the style guide
• My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

[fikretellek]

server is live on ---> https://module-servers.onrender.com/

[musayuksel]

Nice touch using nodemon! This can save you a lot of time and effort during development. Well done!

[musayuksel]

One suggestion for improvement is that instead of using res.sendStatus(400).send("wrong input"), you can use res.status(400).send("wrong input"). res.status(400) set the status code and then send the error message using send.

[fikretellek]

Ahh I see, that makes more sense. Thank you.

[musayuksel]

If you want to go one further step, you can create your validation fun and give single responsibility to it. It looks like this:

function validateMessage(newMessage) {
     //your validation returns boolean
 }

app.post("/messages", (req, res) => {
  const newMessage = req.body;

  const isMessageValid = validateMessage(newMessage

  if (!isMessageValid)) {
    res.status(400).send("wrong input");
    return;
  }
// Rest of your code...

[fikretellek]

I have modified the route function. Could you have a look please?

[musayuksel]

There is no right and wrong, but you generally don't need to include the word 'id' in the route path. you can simply use /messages/:id. It makes the endpoint more concise and follows a common convention in RESTful APIs.

[fikretellek]

I really had a bad time fixing this 😆 when I tried to create another route (ex: /message/search) it was getting recognised as :id parameter. I couldn't solve the problem and found this solution. After seeing this message spent an hour but still couldn't solve 😆

[musayuksel]

Same as before!

[fikretellek]

Can we set up a meeting for this issue? I couldn't find how to resolve dynamic and static path blocking

[musayuksel]

I'm sure you can to this more dynamic instead of using hardcoded length 💯

[fikretellek]

yes it makes more sense. I have updated the route with /messages/latest/:length route

[musayuksel]

Hey @fikretellek ,

This is fantastic! Moving the validation logic to a separate function has already made the code significantly more readable and easier to test.

Here's an additional step you can explore to further improve the structure and maintainability of the code: it's called middleware. Don't worry, it's not your responsibility at this point, but I can see your potential and I'd love to introduce you to it for when you have some extra time to learn.

Middleware in Express allows you to create reusable functions that intercept incoming requests and perform actions before they reach the route handler. This is a great way to for common tasks like validation, authentication.

Here's an example of how you could implement message validation as middleware:

const messageValidationMiddleware = (req, res, next) => {
  const message = req.body;
  const isMessageValid = validateMessage(message);
  if (isMessageValid) {
    next(); // Pass it to the next middleware or route handler in your scenario
  } else {
    const errorMessage = new Error('Your message input is invalid :( ');
    next(errorMessage); // Pass the error to the global error handler
   // IT WILL SKIP ALL OTHER MIDDLEWARES AND ROUTES
  } 
};

// Now, in your route handler, you can remove the validation logic:

app.post('/messages', messageValidationMiddleware, (req, res) => {
  const newMessage = req.body;
  // Since the message has already been validated, we can safely assign an ID
  newMessage['id'] = messages[messages.length - 1].id + 1;
  // ... rest of your code
});

// ... OTHER ROUTES

const globalErrHandler = (err, req, res, next) => {
  res.status(400).send(err.message); // comes from any middleware,
// in this code it is `Your message input is invalid :(`
  // The status code can be dynamic in real applications, but that is beyond the scope of this explanation
};

app.use(globalErrHandler);

app.listen(process.env.PORT, () => {
  console.log(`listening on PORT ${process.env.PORT}...`);
});

• The globalErrHandler function takes four arguments: err, req, res, and next.
• When we can call next(err) in one of our middleware function, it will skip any remaining middleware and route handlers and jump straight to the globalErrHandler middleware.
• The globalErrHandler receives the error object as the first argument (Error).
• By utilising the globalErrHandler, we ensure std error handling throughout our app.

Remember, this is just an optional step you can explore for further learning. The current implementation is already great! Keep up the excellent work, @fikretellek .

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud