-
-
Notifications
You must be signed in to change notification settings - Fork 267
Glasgow Class 6 - HERISH TURKI - JS Core 2 - Week 1 #221
base: main
Are you sure you want to change the base?
Changes from all commits
9dd72d1
42a4a3c
c710e61
0524aee
6274bf3
6c71f3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
|
|
||
| The createShoppingList function should return an object with two properties: | ||
| - "name" of the recipe, which is a string, | ||
| - "items", which is an arry of the missing ingredients that need to be on the shopping list | ||
| - "items", which is an array of the missing ingredients that need to be on the shopping list | ||
| */ | ||
|
|
||
| let pantry = { | ||
|
|
@@ -20,6 +20,21 @@ let pantry = { | |
|
|
||
| function createShoppingList(recipe) { | ||
| // write code here | ||
|
|
||
| let missingIngredients = []; | ||
|
|
||
| // 2 lovely array methods which I learned here | ||
|
|
||
| const allPantryContents = Object.values(pantry).flat(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a neat way to get the ingredients in the pantry into one list! Note that it wouldn't work if the pantry object had other properties too that didn't contain lists of ingredients. But if we know that it never will (and that might be quite reasonable here) then this is a good way to go. |
||
|
|
||
| for (let i = 0; i < recipe.ingredients.length; i++) { | ||
| if (!allPantryContents.includes(recipe.ingredients[i])) | ||
| missingIngredients.push(recipe.ingredients[i]); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works fine, but it would also be quite a good opportunity to use const missingIngredients = recipe.ingredients.filter(
(ingredient) => !allPantryContents.includes(ingredient)
); |
||
| return { | ||
| name: recipe.name, | ||
| items: missingIngredients, | ||
| }; | ||
| } | ||
|
|
||
| /* ======= TESTS - DO NOT MODIFY ===== | ||
|
|
@@ -43,11 +58,18 @@ test("createShoppingList works for pancakes recipe", () => { | |
| test("createShoppingList works for margherita pizza recipe", () => { | ||
| let recipe2 = { | ||
| name: "margherita pizza", | ||
| ingredients: ["flour", "salt", "yeast", "tinned tomatoes", "oregano", "mozarella"], | ||
| ingredients: [ | ||
| "flour", | ||
| "salt", | ||
| "yeast", | ||
| "tinned tomatoes", | ||
| "oregano", | ||
| "mozarella", | ||
| ], | ||
| }; | ||
|
|
||
| expect(createShoppingList(recipe2)).toEqual({ | ||
| name: "margherita pizza", | ||
| items: ["flour", "yeast", "mozarella"] | ||
| items: ["flour", "yeast", "mozarella"], | ||
| }); | ||
| }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These functions are fine and clear - it's generally not advised though to change the value of an argument passed in to a function unless that is the explicit purpose of the function. It happens that we expect a number to be passed in here, which won't cause problems to any code calling these functions. In general, though, if we are building a value to be returned, it's best to declare a new variable to hold this new value as we are building it rather than modifying an argument (here |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all fine! |
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.
The approach here is fine, and it's really good that you've been able to use array restructuring here to write simpler and more comprehensible code!
The format of
COUNTRY_CURRENCY_CODES_OBJECTwould more usually be written ascountryCurrencyCodeObjectin JavaScript - the capital letters joined by underscores format is more for constants that don't change their values during the lifetime of the program, rather than (in this case) a temporary variable for an object that is being built up and then returned. But that's a minor comment.