Skip to content
This repository was archived by the owner on Jan 3, 2023. It is now read-only.

London Class 7 - Somayeh Karimi - JavaScript - core 2 - Week 1 - #45

Closed
DibaKarimi wants to merge 1 commit into
CodeYourFuture:mainfrom
DibaKarimi:main
Closed

London Class 7 - Somayeh Karimi - JavaScript - core 2 - Week 1#45
DibaKarimi wants to merge 1 commit into
CodeYourFuture:mainfrom
DibaKarimi:main

Conversation

@DibaKarimi

@DibaKarimi DibaKarimi commented Feb 20, 2021

Copy link
Copy Markdown

Your Details

  • Your Name: Somayeh
  • Your City: Karimi
  • Your Slack Name: Somayeh Kr

Homework Details

  • Module:Javascript - core 2
  • Week: 1

Comment thread mandatory/5-recipes.js

let recipes = {};
let recipes = [
{

@louisechow louisechow Feb 24, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You did a great job defining the list as an array and then each recipe as an object.

One suggestion would be to always use lowercase when naming your properties - that's the usual convention and you'll see it done that way consistently in documentation/tutorials etc.

So instead of Title, Serves, Ingredients have title, serves, ingredients.

Comment thread mandatory/5-recipes.js
Ingredients: ["coffee", "milk", "honey"],
},
];
recipes.forEach((Element)=>{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What would be a more descriptive name for the variable instead of Element?

Comment thread mandatory/5-recipes.js
console.log(Element.Title);
console.log("Serves: " + Element.Serves);
console.log("Ingredients:");
for (Ingredient in Element.Ingredients) console.log(Element.Ingredients[Ingredient]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since ingredients is an array, you should use for... of... instead of for...in.... for...in... is usually used for objects.
https://betterprogramming.pub/what-is-the-difference-between-for-in-and-for-of-in-javascript-650952654e97?gi=e7681d52d21d

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would also suggest that you format line 63 differently and use curly braces so it's easier to read.
e.g. if I used a for...of... loop it would look like:

for (Ingredient of Element.Ingredients) {
    console.log(Ingredient);
}

let books = [];
let books = [
{
title: "The Hobbit",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make sure you have the same level of indentation for each of the properties in the array.

**/

let books = [];
let books = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You've done a good job defining the array and the objects inside it. You also chose good property names and used correct casing.

isRead:false
}
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

for...in... is usually used for objects. You should use for..of... for arrays.

You can read more about it here
https://medium.com/better-programming/what-is-the-difference-between-for-in-and-for-of-in-javascript-650952654e97

Comment thread mandatory/3-groceries.js
*/
// Gather all week item names into this array
let weeklyGroceriesToBuy = [];
let weeklyGroceriesToBuy = Object.values(weeklyMealPlan).join();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does gather all of the items, but it stores them as a string. Can you think of a solution that gathers the items as an array?

Your list also contains duplicates -- for example Cheese appears twice. How could we remove those?

Comment thread mandatory/3-groceries.js
// Gather weekend item names into this array
let weekendGroceriesToBuy = [];

let weekendGroceriesToBuy = weeklyMealPlan.saturday;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This works for this specific solution, but what if we update the meal plan to also include items on sunday? Also how would we make sure that there are no duplicated items?

friend.friends.find((Element) => Element.name === "Stacie Villarreal")
)
.map((friend) => Object.values(friend.name))
.map((friend) => friend.join(" "))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good solution! But I think (name) => name.join(" ") would be a clearer way to name your variable here.


*/

let arr = [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is arr for?

people
.filter((friend) =>
friend.friends.map((friendName) => {
if( friendName.skills.includes("Multi-tasking"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good solution! Can you think of a solution using filter instead of an if statement?

@github-actions

Copy link
Copy Markdown

Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback.

@github-actions github-actions Bot added the Stale label May 14, 2021
@github-actions github-actions Bot closed this May 14, 2021
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