-
Notifications
You must be signed in to change notification settings - Fork 892
18q Solution #118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 18q
Are you sure you want to change the base?
18q Solution #118
Conversation
|
|
||
| // If a search exists in the URL parameters, | ||
| // filter the products that match the search. | ||
| if (search) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it like this:
if (search) {
search = search.toLowerCase()
filteredProducts = products.filter(product => {
return product.name.toLowerCase().includes(search) ||
product.keywords.includes(search);
})
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same thing and it worked. U are smart, that's nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is mine efficient like his ? :
if(key){
const searchTerm = key.toLowerCase();
filteredProducts = products.filter((productData)=>{
const searchData = (productData.name + productData.keywords.join(' ')).toLowerCase();
console.log(searchData)
return searchData.toLowerCase().includes(searchTerm)
})
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antaugustol
Your code would be correct if "const search" is changed to "let search"...Otherwise, it will throw an error saying constant variable can't be changed.
@SuperSimpleDev 's code throws it's own sets of errors. In the best case that it works, it is case sensitive meaning if you searched for "cover" it will show results but if you search "Cover", there will be zero matches...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it like this:
if (search) {
search = search.toLowerCase()
filteredProducts = products.filter(product => {
return product.name.toLowerCase().includes(search) ||
product.keywords.includes(search);
})
}
it says you can't redeclare search so do this instead:
if (search) {
const searchLower = search.toLowerCase()
filteredProducts = products.filter((product) => {
return product.name.toLowerCase().includes(searchLower) ||
product.keywords.includes(searchLower)
})
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your second version is definitely the better approach. Creating a separate searchLower variable keeps things clear and avoids modifying the original search value. It also prevents the redeclaration issue and makes the filter logic easier to maintain.
|
Great points from everyone. Normalizing the search term with a separate searchLower variable is definitely the cleanest and most reliable approach. It keeps the code predictable, avoids redeclaration issues, and handles case-insensitive matching properly. |
No description provided.