Conversation
|
|
||
| const getProductById = (id) => { | ||
| let product | ||
| for (let n = 0; n < DATA.products.length; n += 1) { |
There was a problem hiding this comment.
It's standard practice to use the variable name i in a for loop.
| @@ -0,0 +1,13 @@ | |||
| import DATA from '../DATA' | |||
|
|
|||
| const getProductById = (id) => { | |||
There was a problem hiding this comment.
The expected function signature is:
const getProductById = (data, productId) => {
There was a problem hiding this comment.
You also need to test to make sure data, data.products, and productId are not null.
| @@ -0,0 +1,13 @@ | |||
| import DATA from '../DATA' | |||
There was a problem hiding this comment.
You don't need to import the data, since the test will pass it in as the first parameter.
| return null | ||
| } else { | ||
| DATA.users.forEach((u) => { | ||
| if (u.userId === DATA.id) { |
There was a problem hiding this comment.
You don't want to test against DATA.id. What is the correct comparison?
if (u.userId === ???) { // <-- What do we really want to compare `u.userId` against?
| @@ -0,0 +1,13 @@ | |||
| import DATA from '../DATA' | |||
There was a problem hiding this comment.
You don't need to pass in the data. The first parameter of all of your functions should be the data.
| @@ -0,0 +1,13 @@ | |||
| import DATA from '../DATA' | |||
|
|
|||
| const getActiveUsers = (id) => { | |||
There was a problem hiding this comment.
Add a data parameter as the first parameter.
There was a problem hiding this comment.
Sanity check the parameters according to the getActiveUsers.test.js file.
| @@ -0,0 +1,17 @@ | |||
| import DATA from '../DATA' | |||
| import DATA from '../DATA' | ||
|
|
||
| const getMostExpensiveProduct = (data) => { | ||
| if (data == null) { |
There was a problem hiding this comment.
This check is correct, but you need one more I think.
| if (data == null) { | ||
| return null | ||
| } | ||
| let mostExpensiveProduct = 0 |
There was a problem hiding this comment.
There are many ways to solve this one. You need to declare a variable like you did, but let's not initialize it: let mostExpensiveProduct
| return null | ||
| } | ||
| let mostExpensiveProduct = 0 | ||
| DATA.products.forEach((m) => { |
There was a problem hiding this comment.
The forEach loop passes in the actual object as the first param to your function. Instead of calling it m, call it currentProduct
| } | ||
| let mostExpensiveProduct = 0 | ||
| DATA.products.forEach((m) => { | ||
| const currentProduct = data.products[m] |
| let mostExpensiveProduct = 0 | ||
| DATA.products.forEach((m) => { | ||
| const currentProduct = data.products[m] | ||
| if (currentProduct.price > mostExpensiveProduct) { |
There was a problem hiding this comment.
Two things need to change here:
- You should test to see if
mostExpensiveProduct == null. OR: - Compare the prices of the
currentProductand themostExpensiveProduct
If either of those are true, enter your if
| return null | ||
| } | ||
| const orderArray = [] | ||
| DATA.orders.forEach((o) => { |
There was a problem hiding this comment.
Replace the o parameter with currentOrder
| } | ||
| const orderArray = [] | ||
| DATA.orders.forEach((o) => { | ||
| const currentOrder = DATA.orders[o] |
|
I added lots of comments to your code! Take a look. If you're confused in any way, please reach out. |
2 participants
No description provided.