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

London 10 | Abubakar-Meigag | JS 2 - wk 1 #251

Open
Abubakar-Meigag wants to merge 4 commits intoCodeYourFuture:mainfrom
Abubakar-Meigag:main
Open

London 10 | Abubakar-Meigag | JS 2 - wk 1 #251
Abubakar-Meigag wants to merge 4 commits intoCodeYourFuture:mainfrom
Abubakar-Meigag:main

Conversation

@Abubakar-Meigag
Copy link
Copy Markdown

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: Abubakar-Meigag
  • Your City: London
  • Your Slack Name: Beko Meigag

Homework Details

  • Module: JS 2
  • Week: 1

Notes

  • What did you find easy?
    exersies
  • What did you find hard?
    write test
  • What do you still not understand?
    write test
  • Any other notes?

@Abubakar-Meigag Abubakar-Meigag added the review requested I would like a mentor to review my PR label Mar 26, 2023
Comment thread 2-mandatory/1-recipes.js
@@ -22,4 +22,21 @@
You should write and log at least 5 recipes
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice 👍

@@ -17,10 +17,36 @@ const COUNTRY_CURRENCY_CODES = [
["MX", "MXN"],
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me! I like your use of the forEach array method here.

Comment thread 2-mandatory/3-shopping-list.js Outdated
for (let ingredient of recipe.ingredients)
{

if (pantry.fridgeContents.includes(ingredient)=== 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.

When you have code that looks like this: pantry.fridgeContents.includes(ingredient) === false - you can rewrite it in this way: !pantry.fridgeContents.includes(ingredient). Can you think of why that works?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done


function createShoppingList(recipe) {
// write code here
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The implementation looks good to me. For an extra challenge - can you implement this using the filter array method?

@@ -20,9 +20,23 @@ const MENU = {
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perfect!

The rest of the tests have comments describing what to test and you need to
write a matching test
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These tests look good 👍

@@ -89,3 +124,12 @@ function formatCourseworkResult(trainee) {
subjects: ["HTML", "CSS", "Databases"]
}
*/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These tests look good - but have a think about the best way to name the tests.
Imagine we make a code change which makes this test fail. In the test output - we will see Trainee Aman, score of but this doesn't really tell us which part of the code is broken. Maybe a better name for this is should return error when coursework score is missing (just an example).

@moneyinthesky
Copy link
Copy Markdown

Nice work on this coursework @Abubakar-Meigag 😄

@moneyinthesky moneyinthesky added the reviewed A mentor has reviewed this code label Mar 31, 2023
@Abubakar-Meigag
Copy link
Copy Markdown
Author

Nice work on this coursework @Abubakar-Meigag 😄

Thanks for you comment and advice I'll start working in all comment thanks again @moneyinthesky

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

Labels

review requested I would like a mentor to review my PR reviewed A mentor has reviewed this code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants