London 9-Lovelace-Yigit Senoglu- JavaScript-Core-2-Coursework-Week1 - #188
London 9-Lovelace-Yigit Senoglu- JavaScript-Core-2-Coursework-Week1 #188ys35code wants to merge 12 commits into
Conversation
maxf
left a comment
maxf
left a comment
There was a problem hiding this comment.
Excellent. You've got this.
At this point, the comments I put are "extras" in order to make the code even better, but they don't matter so much as you've got the main questions almost perfect.
Just one thing: don't forget to complete the README
| UnitedKingdom: "London", | ||
| China: "Beijing", | ||
| Peru: "Lima" | ||
| Peru: "Lima", |
There was a problem hiding this comment.
While trailing commas don't make a difference to your object definition, they have their pros and cons. In the end it's often down to the JS style guide your project will use, but it's good to understand the consequences
There was a problem hiding this comment.
Here (and below) for instance, you probably don't want to add them, because not only they weren't there in the first place and there's probably a reason, but more importantly they introduce changes in your PR that aren't related to what the PR is about. So it makes the "diff" longer by adding unrelated changes.
| // write code here | ||
| } | ||
| getName: function (name) { | ||
| console.log("Student name: " + "" + name); |
There was a problem hiding this comment.
slightly better to use string interpolation here
| }; | ||
|
|
||
| console.log(cake.title); | ||
| console.log("Serves:" + " " + cake.servings); |
There was a problem hiding this comment.
again, string interpolation is preferred. It's more readable.
| console.log("Serves:" + " " + cake.servings); | ||
| console.log("Ingredients:" + " " + cake.ingredients.join(", ")); | ||
|
|
||
| let yogurt = { |
There was a problem hiding this comment.
If you're not going to change your object, use const. It's a good habit to have and leads to safer code
| function createLookup(countryCurrencyCodes) { | ||
| // write code here | ||
| const lookup = {}; | ||
| for (const [country, currency] of countryCurrencyCodes) { |
There was a problem hiding this comment.
nice one. Object.fromEntries is a nice little function you can also use here.
|
|
||
| let players = basketballTeam.topPlayers; | ||
| players.sort(); | ||
| players.forEach(function (topPlayers) { |
There was a problem hiding this comment.
This works as intended, but 2 small things:
- the name of the parameter
topPlayersis misnamed. It will contain the name of a single player. So it's better namestopPlayerorplayerorplayerName. It's important (maybe not here, but in more complex code) to use variable and parameter names that reflect as precisely as possible what they're going to contain. - you can use the arrow notation to make your code shorter and easier to read
| function createShoppingList(recipe) { | ||
| // write code here | ||
| const shoppingList = { name: recipe.name, items: [] }; | ||
| for (const ingredient of recipe.ingredients) { |
There was a problem hiding this comment.
have you run the tests? ingredient will change at each iteration so using const will give you an error
| ingredients: [ | ||
| "flour", | ||
| "salt", | ||
| "yeast", | ||
| "tinned tomatoes", | ||
| "oregano", | ||
| "mozarella", | ||
| ], |
There was a problem hiding this comment.
You know what I'm going to say here :)
Try not to change the parts of the code you're not asked to change to keep the commits focused on the task at hand, and not formatting changes for the code you're given.
If you think that code needs to be changed for readability (and in this case I agree) do it in a separate commit labeled "code cleanup" or something like that. And make a real PR so that the source repository gets fixed!
3 participants
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?