Skip to content
This repository was archived by the owner on Oct 2, 2024. It is now read-only.

Conversation

@ElenaBarker
Copy link

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

  • Your Name:
  • Your City:
  • Your Slack Name:

Homework Details

  • Module:
  • Week:

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

@ElenaBarker ElenaBarker changed the title London 10 - Elena Barker - Node - Week 1 London 10 - Elena Barker - Node Module - Quote-Server - Week 1 Jul 18, 2023
Copy link

@giorgiguts giorgiguts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work! I think you will find a couple of comments which I've added quite useful.


app.get("/quotes/search", function (request, response) {
const term = request.query.term;
if (!term) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent use of negation! 🥳 Instead of writing unnecessary expressions such as if(term === undefined) with negation, we as devs can really shorten the if expressions and make it really understandable for other devs looking at it!

server.js Outdated

//START OF YOUR CODE...
app.get("/quotes", function (request, response) {
response.json(quotes);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work! As an extra layer of security and with the idea of minimising the amount of bugs in our backend and in our codebase in general, we can always think of adding an extra layer of protection.

For instance, whenever you send a response, it's generally a good idea to add a check that our response came back with a code 200, which means that the request is successful (you can read more on http status codes here).

If you are using express.js v4 and higher, you can simply add this line:
response.status(200).json(quotes);
This will check that if the response is 200, only after it will send the quotes json 😊

Otherwise, overall, excellent work!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the review, Giorgi. I truly appreciate your valuable feedback, and I have integrated your recommendations to improve my code.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Elena, it's my pleasure and I'm glad if you found my small recommendation useful! 😊 keep up the great work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants