London | 25-ITP-Sep | Imran Mohamed | Sprint 1 | Feature/destructuring#327
London | 25-ITP-Sep | Imran Mohamed | Sprint 1 | Feature/destructuring#327i786m wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
i786m
added
the
Needs Review
ckirby19
added
Review in progress
ckirby19
left a comment
ckirby19
left a comment
There was a problem hiding this comment.
Great work! I just added come comments around readability
| //task 1 | ||
| function getGryffindorMembers(arr) { | ||
| let gryffindorMembers = []; | ||
| arr.forEach(({ firstName, lastName, house }) => { |
There was a problem hiding this comment.
As an extension: could you show me how you can use filter and map on an array to do this?
There was a problem hiding this comment.
Used filter and map for both functions in this exercise
| function printReceipt(order){ | ||
| let total = 0 , qtyWidthMax = 3, itemWidthMax = 4 | ||
| const receiptLines = order.map(({itemName, quantity, unitPricePence}) => { | ||
| const itemTotalPounds = (quantity*unitPricePence/100); |
There was a problem hiding this comment.
Generally, we try to keep space around an operator to make the code easier to read. Also, just to make the code easier to read, it is better to use brackets to show the order of operation that you want. For example, is it not clear whether you intend to have:
- quantity * (unitPricePence/100) OR
- (quantity * unitPricePence) / 100
In this instance, it does not actually matter, as mathematically they are equivalent. But it is a good habit to create to improve your code readability
| ]; | ||
|
|
||
| function printReceipt(order){ | ||
| let total = 0 , qtyWidthMax = 3, itemWidthMax = 4 |
There was a problem hiding this comment.
Just formatting: We usually don't add a comma after a number like you have with total = 0 ,
There was a problem hiding this comment.
Noted, have declared and assigned variables individually to aid in readability.
ckirby19
added
Reviewed
…e filter and map for improved readability
i786m
added
Needs Review
|
|
||
| //task 1 | ||
| function getGryffindorMembers(arr) { | ||
| const gryffindorMembers = arr.filter(({ house }) => house === "Gryffindor").map(({ firstName, lastName }) => `${firstName} ${lastName}`).join('\n'); |
There was a problem hiding this comment.
Nice one, great work. The only thing I would say is that we typically keep each method applied on a new line just to improve readability again. So, to take your work as an example:
ckirby19
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
I have completed the tasks set out in exercises 1-3 in the Sprint 1 destructuring folder
Questions
I found it unclear if exercise 2 needed a string output, or a log as the task was to display the desired names. As such I have logged these to the console, should I have been outputting strings instead?