-
-
Notifications
You must be signed in to change notification settings - Fork 445
London Class 9 - Natalie Zablotska - Node - Week 2 #275
base: master
Are you sure you want to change the base?
Conversation
|
classwork/server-2.js
Outdated
}); | ||
|
||
app.delete("/people", (request, response) => { | ||
res.send("DELETE Request Called"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to try to use VSCode's code formatting function (press CMD/CTRL + P, then enter "> format" at the prompt box)
Are you deleting an element from the people
array? Did you miss capturing the id from url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for noticing - I've finished it now! Didn't know about CTRL + P + '> format', will learn more about this toll now. Thank you!
classwork/server-2.js
Outdated
app.listen(3000, () => { | ||
console.log("listening on port 3000"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assign port number in a variable so you can refer to it in your console message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Jack, done :)
res.status(200).send({ messages: latestMessages }); | ||
}); | ||
|
||
app.put("/messages/:id", function (req, res) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if no message with the specified id is present? Also, what if the provided "id" is not an integer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, didn't think about it. Added if (messageIndex === -1) { return res.status(404).send({ error: "Message not found" });}
and
if (isNaN(id)) {return res.status(400).send({ error: "Invalid ID" });}
Do you think it's a good way to address these issues, or is there a better way?
// const PORT = 9090; | ||
// app.listen(process.env.PORT); | ||
|
||
app.listen(9090, () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can listen to port specified by process.env.port
when it is present or port 9090 by saying
app.listen( process.env.port || 9090,
Again, its better to use a variable to store the default port number 9090
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, thanks for reviewing the code, Jack!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont have definite answers to your question but just want to highlight the importance of
- Input validation
- Error handling
I think Alun's material provided examples on the above aspects. You may take a look
No description provided.