2
\$\begingroup\$

I built a search bar that will filter an array of objects. But I feel like I can improve it. When I filter, I want to filter the first and last name case insensitive, then sort the names and then map them to be rendered.

I'm using typescript so ignore SelectedProps:

return contacts
    .filter((item: SelectedProps) => (item.firstName + item.lastName).toLowerCase().includes(search.toLowerCase()))
    .sort((a: SelectedProps, b: SelectedProps) => a.firstName > b.firstName ? 1 : -1)
    .map((item: SelectedProps, index: number) => (
     // IGNORE OUTCOME 
    ));
\$\endgroup\$
  • 1
    \$\begingroup\$ Besides using localeCompare for your sort function, I probably wouldn't change anything here. What are you looking for in answers? \$\endgroup\$ – Gerrit0 Jul 13 '17 at 23:19
  • \$\begingroup\$ To see if this would be acceptable in production! If it looks good, then I would leave as it is. \$\endgroup\$ – A K Jul 13 '17 at 23:20
  • \$\begingroup\$ This looks good to me. Honestly I wouldn't change anything, but if the code itself is too bulky for your taste you can pull the sort fn and this bit (item.firstName + item.lastName).toLowerCase() into named utility functions like loweredName(item) and sortByFirstName, and use those within this code. \$\endgroup\$ – Jonah Jul 14 '17 at 4:38
1
\$\begingroup\$

If someone searches for "Firstname Lastname" (with a space) it won't find it, even if it exists. I would suggest removing spaces from the search string with search.replace(/ /g, '')

You are not doing anything for special characters like apostrophe. Should searching for "OConner" not return "O'Connor"?

The sort function you have is almost the same as the default sort, except yours doesn't return 0 if they are equal. But you shouldn't use that anyway, since they are case sensitive. You can use .localeCompare instead.

Also note the IE doesn't support .includes. If this is running in a browser and you're not already using a polyfill or transpiler, you should do that, or find another way if you want to support IE.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.