Having some trouble with a code I'm trying to refactor, where I need to go over a 'tree' of items, filter some invalid items, and create a new array of filtered items which takes properties from both parent and child objects.
To demo the problem I made a quick snippet -
const items = [
{
name: 'Shirley Hardin',
isValid: true,
children: [
{
name: 'Mildred Mata',
isValid: true,
},
{
name: 'Christopher Herring',
isValid: false,
},
],
},
{
name: 'Tad M. Colbert',
isValid: false,
children: [
{
name: 'James J. Spencer',
isValid: true,
},
],
}
];
function toParentChildModel(parent, child) {
return {
parentName: parent.name,
childName: child.name,
};
}
function getValidChildParent(list) {
const validParentChild = [];
for (let i=0; i<list.length; i++) {
const parent = list[i];
if (parent.isValid) {
for (let j=0; j<parent.children.length; j++) {
const child = parent.children[j];
if (child.isValid) {
validParentChild.push(toParentChildModel(parent, child));
}
}
}
}
return validParentChild;
}
function print(list) {
console.log(list);
}
print(getValidChildParent(items));
The code can also be found at here
How would you refactor this code, specifically the 'getValidChildParent' function, in a functional way?
2 Answers 2
getValidParentAndChild
can be rewritten as follows:
function getValidParentAndChild(list){
return list.filter((obj) => obj.isValid)
.map((obj) => obj.children.filter((kid) => kid.isValid)
.map((kid) => toParentChildModel(obj, kid))
.reduce((arr1, arr2) => arr1.concat(arr2));
}
The filter
takes the element of the list and keeps it if the condition passed in is met. (In this case, both uses keep all the valid elements.)
map
applies a function over each element of the list and returns the list of what the function returned for each element. Here it converts the list to a list of the parent-child pairs.
reduce
is also called fold
. These apply a two-argument over a list such that if your list was [a, b, c, d]
, with the function f
, the reduce
would return f( f( f(a, b), c), d)
. Here it is used to make a 2D list a 1D array.
This problem becomes more interesting if children could have children. In that case I would do the following:
function getPairsForNode(node){
if(node.children)
return node.children.map((child) => getPairsForNode(child))
.concat(node.children.map((child) => toParentChildModel(node, child))
.reduce((arr1, arr2) => arr1.concat(arr2));
else return [];
}
function getValidParentAndChild(list){
return list.filter((node) => node.isValid)
.map(getPairsForNode)
.reduce((arr1, arr2) => arr1.concat(arr2));
}
Interesting question,
since Heman addressed what you wanted to have reviewed, I will take a stab at the rest:
isValid
seems like a very bad naming choice, having it on bothparent
andchild
does not help at all. There must be some logic to determine the validity and that logic should be reflected in the name of the variable.If however, that is really the way it should be, then you could enhance a tiny bit Heman's code by having a dedicated
isValid
function.function isValid( o ){ return o.isValid; } function getValidParentAndChildNames(list){ return list.filter(isValid) .map((parent) => parent.children.filter(isValid) .map((child) => toParentChildModel(parent, child))) .reduce((previous, current) => previous.concat(current)) }
I would also suggest that
getValidParentChildNames
is a better name thangetValidParentAndChild
, since you are getting an array back (ends withs
) and since you are only getting the names back, not the actual nodesIf you follow that logic thru, then consider also renaming
toParentChildModel
model, and then you can even consider passing the names straight to the function (why should the function only work for your model? If your function got the names straight, then it would be far more re-usable) The final value would be that you could then use a great ES6 feature:return { parentName, childName, };
Explore related questions
See similar questions with these tags.