On my app I'm checking the equality between an objects array property and a nested object property - based on the results I return a new objects array.
The items
objects array:
[{name: 'test', value: 1, category: 'a'},{name: 'test2', value: 2, category: 'b'},{name: 'test3', value: 3, category: 'c'}]
The itemList
object:
{testnum1: {name: 'test123', category: 'a'}, testnum2: {name: 'test234', category: 'b'}
I need to check the equality between the category
properties and return the following objects array:
[{name: 'test', itemCategory: 'testnum1'},{name: 'test2', itemCategory: 'testnum2'}]
This is what I did, any idea on how to make it more readable?
const getCategorizedItems = items => {
const itemListCategories = Object.keys(itemList)
const categorizedItems= items.map(item=> {
const itemCategory = itemListCategories.find(category =>
item.category === itemList[category].category
)
if (itemCategory ) return {name: item.name, category: itemCategory }
})
.filter(f => f)
return categorizedItems
}
-
2\$\begingroup\$ Welcome to CodeReview@SE. Please add to your question how the code presented is going to be used: What is it good for? (Re)Visit How do I ask a Good Question? \$\endgroup\$greybeard– greybeard2021年01月12日 10:09:48 +00:00Commented Jan 12, 2021 at 10:09
3 Answers 3
Short review;
- Unless you are writing inline functions, you should stick to using
function
- Using semicolons is preferred, it takes skill and knowledge to avoid the pitfalls of not using semicolons
- You assign in 1 statement the value of
categorizedItems
andreturn
it in the next, you might as wellreturn
immediately - In essence you do a
find
within amap
, not too fast, I would create a lookup structure before themap
itemList
seems like a terrible name, I would call itcategories
I would counter-propose the following;
const items = [{name: 'test', value: 1, category: 'a'}, {name: 'test2', value: 2, category: 'b'}, {name: 'test3', value: 3, category: 'c'}];
const categories = {testnum1: {name: 'test123', category: 'a'}, testnum2: {name: 'test234', category: 'b'}};
function getCategorizedItems(items, categories){
//Desired output; [{name: 'test', itemCategory: 'testnum1'},{name: 'test2', itemCategory: 'testnum2'}]
const categoriesMap = Object.fromEntries(Object.entries(categories).map((entry) => [entry[1].category, entry[0]]));
return items.map(item => ({name: item.name, itemCategory:categoriesMap[item.category]})).filter(item => item.itemCategory);
}
console.log(getCategorizedItems(items, categories));
I would start by extracting the relevant data from itemList
and create a lookup object that maps the category
to the relevant key/itemCategory.
const itemCategories = new Map(
Object.entries(itemList)
.map(([itemCategory, {category}]) => [category, itemCategory])
);
With the above structure you can filter
the items
and only keep the items that have a category
property that is present within the lookup object. After filtering the array you can map
the data into the desired result.
items
.filter(item => itemCategories.has(item.category))
.map(item => ({
name: item.name,
itemCategory: itemCategories.get(name.category)
}));
const items = [{name: 'test', value: 1, category: 'a'}, {name: 'test2', value: 2, category: 'b'}, {name: 'test3', value: 3, category: 'c'}];
const itemList = {testnum1: {name: 'test123', category: 'a'}, testnum2: {name: 'test234', category: 'b'}};
const itemCategories = new Map(
Object.entries(itemList)
.map(([itemCategory, {category}]) => [category, itemCategory])
);
function getCategorizedItems(items) {
return items
.filter(item => itemCategories.has(item.category))
.map(item => ({
name: item.name,
itemCategory: itemCategories.get(item.category)
}));
}
console.log(getCategorizedItems(items));
There is some very bad naming in the code. Names are too long and have overlapping abstractions. What is a category, there are two types a
, b
, or c
and testnum1
and itemNum2
Due to poor naming the function has lost so much semantic accuracy it takes a second look to see what it is doing.
What are you doing?
getCategorizedItems
getting categorized items??
From the input example all items are already categorized. So the function is not really getting categorized items
The output has new categories
"a"
becomes"test123"
so the items are being recategorized (or because there is a one to one match the cats are being transformed)Each item is restructured, From
{name: 'test', value: 1, category: 'a'}
to{name: 'test', category: 'testnum1'}
Items are filtered by existence of transformed category names.
Rename
Naming the function transformCategoriesRestructureAndFilter
is impractical so recategorize
could be more apt within the context of the code
As you are using category
as an identifier, you should make it clear categoryId
or catId
, or even just id
. Thus
// ignoring irrelevant data
itemList = {testnum1: {catId: 'a'}, testnum2: {catId: 'b'}};
items = [{name: '1', catId: 'a'},{name: '2', catId: 'b'},{name: '3', catId: 'c'}];
Use appropriate data structures
The object itemList
is a transformation and should be a Map for quick lookup. Either defined as such
const catIdTransform = new Map((["a", "testnum1"], ["b", "testnum2"]]);
Or constructed from itemList
const catIdTransform = new Map(Object.entries(itemList).map(([name, {catId}]) => [catId, name]));
Rewrite
Thus you can rewrite using the new naming.
First transform the catId
and then filter items without new ids.
function recategorize(items, idMap) {
return items.map(item => ({...item, catId: idMap.get(item.catId)}))
.filter(({catId}) => catId);
}
Working example
Using renamed data structures.
const items = [{name: '1', catId: 'a'},{name: '2', catId: 'b'},{name: '3', catId: 'c'}];
const catIdTransform = new Map([["a", "num1"], ["b", "num2"]]);
function recategorize(items, idMap) {
return items.map(item => ({...item, catId: idMap.get(item.catId)}))
.filter(({catId}) => catId);
}
console.log(recategorize(items, catIdTransform));
or if you are forced to maintain the naming and existing data
const items = [{name: '1', category: 'a'}, {name: '2', category: 'b'}, {name: '3', category: 'c'}];
const itemList = {testnum1: {category: 'a'}, testnum2: {category: 'b'}};
const categoryTransform = new Map(Object.entries(itemList).map(([name, {category}]) => [category, name]));
function categorizedItems(items, idMap) { // at least drop the get
return items.map(item => ({...item, category: idMap.get(item.category)}))
.filter(({category}) => category);
}
console.log(categorizedItems(items, categoryTransform));