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

London Class 10 - Elena Barker - JS2 - Week 1#262

Open
ElenaBarker wants to merge 9 commits intoCodeYourFuture:mainfrom
ElenaBarker:main
Open

London Class 10 - Elena Barker - JS2 - Week 1#262
ElenaBarker wants to merge 9 commits intoCodeYourFuture:mainfrom
ElenaBarker:main

Conversation

@ElenaBarker
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: Elena Barker
  • Your City: London
  • Your Slack Name: Elena Barker

Homework Details

  • Module: JS2
  • Week: 1

Notes

  • What did you find easy?

  • What did you find hard? I found tricky to write a tests

  • What do you still not understand?

  • Any other notes?

@ElenaBarker ElenaBarker added the review requested I would like a mentor to review my PR label Apr 8, 2023
*/

// write code here
let players = basketballTeam.topPlayers.sort();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

instead of calling the method just players you could write 'sortedPlayers' to add more of a description to what the method is doing.


let myCountry = "UnitedKingdom";
let myCapitalCity; // complete the code
let myCapitalCity = capitalCities["UnitedKingdom"]; // complete the code
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you do this another way instead of hardcoding?

// write code here
capitalCities.UnitedKingdom.population = 898000;
capitalCities.China.population = 21500000;
capitalCities.Peru = {};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

here you could add the new country and its properties in one statement


// write code here
if (student["attendance"]>= 90 && student["examScore"]) {
student.hasPassed = true;
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'm not sure if line 29 is bracket notation? how else could you write this line?

// write code here
getName: function (name){
console.log(`Student name: ${name}`)
}
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 could have a go using arrow function syntax here instead?

Comment thread 2-mandatory/1-recipes.js
console.log(`Ingredients:`);

for (let i = 0; i<lasagne.ingredients; i++){
console.log(lasagne.ingredients[i])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

here we need '.length' or else the function will not know how many times to loop around. Instead of doing iterations is there another way you are able to loop in javaScript? have a look a the forEach method.

// write code here
const objectCodes = {};
for (element of countryCurrencyCodes){
objectCodes[element[0]] = element[1];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

try not to use words like element as it can make it harder for someone else to read through your code you could use country or currency directly. e.g. instead of element const [country, currency]

const missingFromThePantry = {};
missingFromThePantry.name = recipe.name;
missingFromThePantry.items = [];
missingFromThePantry.items = recipe.ingredients.filter ((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.

instead of declaring an empty object then adding to it we could directly declare the object.

missingFromThePantry.items = recipe.ingredients.filter ((Ingredient) => {
if (!pantry.fridgeContents.includes(Ingredient) && !pantry.cupboardContents.includes(Ingredient)){
return true;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

instead of using the if statement we could use an or function instead within the filter function

let cashRegister = {
// write code here
orderBurger: function (balance) {
let burgerPrice = balance - MENU.burger;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not too important but we could use more descriptive variables so instead of balance could use remainingBalance

@mira-shukla mira-shukla added the reviewed A mentor has reviewed this code label Apr 14, 2023
Copy link
Copy Markdown

@mira-shukla mira-shukla left a comment

Choose a reason for hiding this comment

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

  • Need to add .length to the iterator

Great work :) just some small improvements and the tests were great!

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