London Class 10 - Iryna Lypnyk - Node - Week 1 #319
Conversation
There was a problem hiding this comment.
Think this is really well done! I've added some advanced comments with links to the documentation which you can explore if you want to. As they allow you to write your code in a more concise way but your code is already very readable and easy to understand so well done!
| @@ -3,7 +3,10 @@ | |||
|
|
|||
| //load the 'express' module which makes writing webservers easy | |||
| const express = require("express"); | |||
| const lodash = require('lodash'); | |||
| var cors = require("cors"); | |||
There was a problem hiding this comment.
Would change this to const. It is best practice to avoid using var, the reason for this is that var has unexpected behaviour when it comes to scoping which can make it hard to reason about your program see the mozilla docs on var for more info.
In this particular case it won't matter but it's definitely better to use either let or const as they respect local (lexical) scoping
There was a problem hiding this comment.
I have just seen that this is in the README instructions so I'll raise this with the team as clearly this isn't something you chose, so is unfair to give you feedback on it. I have left the above comment though as it is still worth knowing
| if (request.query) { | ||
| const term = request.query.term; | ||
| if (term) { | ||
| const termCaseSencitive = term.toLowerCase(); |
There was a problem hiding this comment.
slight typo on this variable name: termCaseSencitive -> termCaseSensitive
| const quotesWithTerm = quotes.filter((q) => { | ||
| const { quote, author } = q; | ||
| return ( | ||
| quote.toLowerCase().includes(termCaseSencitive) || | ||
| author.toLowerCase().includes(termCaseSencitive) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
No need for the q variable as you can destruct this in the function argument.
const quotesWithTerm = quotes.filter(({ quote, author }) => {
...
});| if (request.query) { | ||
| const term = request.query.term; | ||
| if (term) { |
There was a problem hiding this comment.
You can avoid the nested if statements by utilising a guard statement and early returning i.e
if (!request.query?.term) {
return;
}
// You now know term is defined so can write the rest of the code without an if statement
// and can immediately make the term variable lower case
const term = request.query.term.toLoweCase();
...The important syntax to note to make this work is the ?. operator which is the optional chaining operator. It allows you to query nested objects as it will return null which evaluates to false if no query was passed in
| if (request.query) { | ||
| const word = request.query.word; | ||
| response.send(`You said ${word}`); | ||
| } |
There was a problem hiding this comment.
As a stretch could you use the optional chaining operator to make it so that if no word is passed in that you respond with a different message informing the user they didn't say a word
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.mdin the root of this repositoryYour Details
Homework Details
Notes
What did you find easy?
What did you find hard?
What do you still not understand?
Any other notes?