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

Glasgow Class 6 - Diana Savchuk - JS-1 - Week 3 #263

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

DianaSavchuk
Copy link

No description provided.

Copy link

@shieldo shieldo left a comment

Choose a reason for hiding this comment

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

This is all really good work! All the tests pass, which is impressive. I did notice that one of the tests at the end had been changed slightly - do remember that tests shouldn't be changed, but actually it was just because of expressing numbers in two decimal places (which you had already done elsewhere), and you'd got looping through the array correct, which is the main thing here!

I'd advise you to look at array methods like .map() and .filter(), and see if you can think how you might be able to simplify some of this code using these methods and callback functions. They may be a little tricky to understand at first, but they do make writing functions like this quite a lot simpler once you are used to them so it's worth your time trying to use them!

@DianaSavchuk
Copy link
Author

Thanks a lot for feedback, Douglas. I will definitely try to implement .map() and .filter() methods in my code :)

@shieldo
Copy link

shieldo commented Mar 11, 2023

This is good work - you've used .map() here to break down the problem into smaller pieces that can be handled with simpler functions that work for one value at a time. 👍

@@ -20,17 +20,17 @@ function sayHello() {
}

let hello = sayHello();
console.log(hello);
console.log(hello); //sayHello() function does not have a return statement

Copy link

Choose a reason for hiding this comment

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

Nice One

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants