Glasgow | 26-ITP-May | Sandani Kannangara | sprint 2 | Data groups - #1236
Glasgow | 26-ITP-May | Sandani Kannangara | sprint 2 | Data groups#1236SandzSoft wants to merge 19 commits into
Conversation
SandzSoft
added
📅 Sprint 2
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work on these tasks, I've left some extra comments for things you could improve.
| }; | ||
|
|
||
| console.log(`${recipe.title} serves ${recipe.serves} ingredients:`); | ||
| recipe.ingredients.forEach((ingredient) => console.log(ingredient)); |
There was a problem hiding this comment.
As a stretch - there is a way to write this last line without an anonymous function. Can you figure it out?
There was a problem hiding this comment.
Hi Leon, Do you mean using a for loop for an array like this,
for (let ingredient of recipe.ingredients) {
console.log(ingredient);
}
There was a problem hiding this comment.
If you want to use array.forEach, there is a way to tell it to use a function, without needing to write => - can you figure out how? It can improve readability a lot.
There was a problem hiding this comment.
I tried recipe.ingredients.forEach(console.log). It removes the anonymous function, but it also prints the index and the array because forEach passes three arguments. Was this the approach you were hinting at, or is there another function-reference pattern you had in mind?
I updated my code with separate print function.
There was a problem hiding this comment.
That's exactly the approach I was thinking of. Whenever you have a function that just calls another function it's usually a good indicator you can simplify things
LonMcGregor
added
Reviewed
SandzSoft
added
Needs Review
SandzSoft
added
Core
SandzSoft
added
Core
LonMcGregor
added
Reviewed
SandzSoft
added
the
Needs Review
SandzSoft
removed
the
Reviewed
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
This works. Just be aware that if you only needed a function to do one thing, you can call it directly in forEach. Of course, having a separate function like this is a bit more maintainable if you wanted to change how things were printed later on. Good work on this task.
| }; | ||
|
|
||
| console.log(`${recipe.title} serves ${recipe.serves} ingredients:`); | ||
| recipe.ingredients.forEach((ingredient) => console.log(ingredient)); |
There was a problem hiding this comment.
That's exactly the approach I was thinking of. Whenever you have a function that just calls another function it's usually a good indicator you can simplify things
LonMcGregor
added
Complete
2 participants
Self checklist
Changelist
Implemented and tested data group functions including tally, invert, lookup, parseQueryString, calculateMode, and totalTill.
Added Jest tests covering normal cases and edge cases, including invalid inputs. Refactored calculateMode into smaller functions and fixed issues with object handling, query string parsing, and frequency counting.