Skip to content

Week 2 Exercise - Initial Commit#16

Open
bbarth86 wants to merge 1 commit intopce-uw-jscript400:masterfrom
bbarth86:master
Open

Week 2 Exercise - Initial Commit#16
bbarth86 wants to merge 1 commit intopce-uw-jscript400:masterfrom
bbarth86:master

Conversation

@bbarth86
Copy link

No description provided.

Copy link
Contributor

@bwreid bwreid left a comment

Choose a reason for hiding this comment

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

Great job!

const { vegetables } = data;
const { name } = req.query;
// Filter array for vegetable objects where vegetable name contains the value provided for req.query.name
// Question: What is the best way to handle case sensitivity and quotes in the query param? Regex?
Copy link
Contributor

Choose a reason for hiding this comment

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

A couple options for this, but here's one:

const { name: unformattedName } = req.query
const sanitized = JSON.parse(unformattedName)
const name = sanitized.toLowerCase()

Then, compare to the vegetable name lowercased as well.

const { name } = req.query;
// Filter array for vegetable objects where vegetable name contains the value provided for req.query.name
// Question: What is the best way to handle case sensitivity and quotes in the query param? Regex?
const filteredVeg = vegetables.filter(veg => veg.name.includes(name));
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this below to your else case. That way you skip over running this if there's no name.

// find the index of vegetable where id matches req.params.id (again, only one match) and update name and price values using req.params.
// then return updated vegetable with 201 response code
let indexOfVeg = vegetables.findIndex(veg => veg.id === id);
vegetables[indexOfVeg] = {id: id, ...req.body}
Copy link
Contributor

Choose a reason for hiding this comment

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

This totally works, but the instructions technically say to throw an error if all of the keys are not present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants