DRY your code You reference element[filterField] quite a few times in filterCallback. It would be nicer to save the value in a standalone variable first - then you can reference that variable instead of doing object property lookup each time.
Nested object bug filterCallback returns a function, but here, if an array or object is found, you're returning a call of filterCallback, which will always be truthy, so the .some will pass, even when it shouldn't:
return filterFields.some((filterField) => {
if (Array.isArray(element[filterField])) {
return element[filterField].some(function(childElement) {
return filterCallback(childElement); // <--- always truthy
})
} else if (typeof element[filterField] === 'object') {
return filterCallback(element[filterField]); // <--- always truthy
}
return comparisonFunction(element[filterField], filterTerm); // <--- OK
})
// No items should pass the test, but they do:
//if filterTerm or filterFields are empty, return original list
function filterList(list, filterTerm, filterFields) {
if (filterTerm && filterFields.length > 0) {
return list.filter(filterCallback(filterFields, filterTerm));
} else {
return list;
}
}
//use closure to pass filterFields and filterTerm to callback
function filterCallback(filterFields, filterTerm) {
return function(element) {
return filterFields.some((filterField) => {
if (Array.isArray(element[filterField])) {
return element[filterField].some(function(childElement) {
return filterCallback(childElement);
})
} else if (typeof element[filterField] === 'object') {
return filterCallback(element[filterField]);
}
return comparisonFunction(element[filterField], filterTerm);
})
}
}
//comparisonFunction - field value is populated, and either filterterm contains the fieldValue or visa-versa
function comparisonFunction(fieldValue, filterTerm) {
return fieldValue &&
(String(fieldValue).toLowerCase().indexOf(filterTerm.toLowerCase()) >= 0 ||
String(filterTerm).toLowerCase().indexOf(fieldValue.toLowerCase()) >= 0);
}
arr2 = [{
name: "a",
job: "b",
employees: [{
name: "z",
job: "e"
}]
}, {
name: "f",
job: "g"
}];
console.log(filterList(arr2, "d", ["name", "employees"])); //=> [{name: "a", job:"b", employees: [{name: "d", job: "e"}]}]
You need to call filterCallback(filterFields, filterTerm) first to get the filter function, then call the filter function with the value to check, eg:
return filterFields.some((filterField) => {
const value = element[filterField];
if (Array.isArray(value)) {
return value.some(function (childElement) {
return filterCallback(filterFields, filterTerm)(childElement);
});
} else if (typeof value === 'object') {
return filterCallback(filterFields, filterTerm)(value);
}
return comparisonFunction(value, filterTerm); // <--- OK
});
//if filterTerm or filterFields are empty, return original list
function filterList(list, filterTerm, filterFields) {
if (filterTerm && filterFields.length > 0) {
return list.filter(filterCallback(filterFields, filterTerm));
} else {
return list;
}
}
//use closure to pass filterFields and filterTerm to callback
function filterCallback(filterFields, filterTerm) {
return function(element) {
return filterFields.some((filterField) => {
const value = element[filterField];
if (Array.isArray(value)) {
return value.some(function (childElement) {
return filterCallback(filterFields, filterTerm)(childElement);
});
} else if (typeof value === 'object') {
return filterCallback(filterFields, filterTerm)(value);
}
return comparisonFunction(value, filterTerm); // <--- OK
});
}
}
//comparisonFunction - field value is populated, and either filterterm contains the fieldValue or visa-versa
function comparisonFunction(fieldValue, filterTerm) {
return fieldValue &&
(String(fieldValue).toLowerCase().indexOf(filterTerm.toLowerCase()) >= 0 ||
String(filterTerm).toLowerCase().indexOf(fieldValue.toLowerCase()) >= 0);
}
//EXAMPLE USE:
arr = [{
name: "a",
job: "b"
}, {
name: "c",
job: "d"
}];
console.log(filterList(arr, "a", ["name"])); // => [{name: "a", job:"d"}]
arr2 = [{
name: "a",
job: "b",
employees: [{
name: "z",
job: "e"
}]
}, {
name: "f",
job: "g"
}];
console.log(filterList(arr2, "d", ["name", "employees"])); //=> [{name: "a", job:"b", employees: [{name: "d", job: "e"}]}]
Precise variable names To reduce bugs, best to name variables as precisely as possible - filterCallback isn't actually the callback used in filter or some, but it's a function which creates such a callback. Maybe call it createFilterCallback instead - that probably would've made the problem clearer at a glance.
Implicit return Since you're using ES6, you could consider using arrow functions too - their implicit return can make code much more concise. Less code means less surface area for bugs, and can sometimes make it easier to read.
comparisonFunction If you want to check whether a string includes another, in ES6, better to use .includes(...) than .indexOf(...) >= 0. You also have a potential bug - you do String(fieldValue).toLowerCase() and String(filterTerm).toLowerCase(), implying that you expect that the value or term may not be a string - but then you do .indexOf(filterTerm.toLowerCase()) and .indexOf(fieldValue.toLowerCase()), which will throw if they aren't strings. Make sure to convert them to strings first:
const comparisonFunction = (fieldValue, filterTerm) => {
const value = String(fieldValue).toLowerCase();
const term = String(filterTerm).toLowerCase();
return value && value.includes(term) || term.includes(value);
};
null is an object too, unfortunately, so:
}else if(typeof element[filterField] === 'object'){
return filterCallback(element[filterField]);
}
will result in a recursive call, even when such a call won't make sense. The code works, but it's confusing flow. Better to only recurse if the value is an object and is not null.
filterCallback The whole filterCallback looks a bit ugly IMO. I'd simplify it a bit by not calling the whole function again recursively, but by declaring the function to filter elements by inside the function before returning it. Then, instead of recursing on filterCallback, recurse on that declared function:
const filterCallback = (filterFields, filterTerm) => {
const testElement = element => filterFields.some((filterField) => {
const value = element[filterField];
if (Array.isArray(value)) {
return value.some(testElement);
} else if (value && typeof value === 'object') {
return testElement(value);
} else {
return value && comparisonFunction(value, filterTerm);
}
});
return testElement;
}
In all:
//if filterTerm or filterFields are empty, return original list
function filterList(list, filterTerm, filterFields) {
if (filterTerm && filterFields.length > 0) {
return list.filter(filterCallback(filterFields, filterTerm));
} else {
return list;
}
}
//use closure to pass filterFields and filterTerm to callback
const filterCallback = (filterFields, filterTerm) => {
const testElement = element => filterFields.some((filterField) => {
const value = element[filterField];
if (Array.isArray(value)) {
return value.some(testElement);
} else if (value && typeof value === 'object') {
return testElement(value);
} else {
return value && comparisonFunction(value, filterTerm);
}
});
return testElement;
}
//comparisonFunction - field value is populated, and either filterterm contains the fieldValue or visa-versa
const comparisonFunction = (fieldValue, filterTerm) => {
const value = String(fieldValue).toLowerCase();
const term = String(filterTerm).toLowerCase();
return value.includes(term) || term.includes(value);
};
//EXAMPLE USE:
arr = [{name: "a", job: "b"}, {name:"c", job:"d"}];
console.log(filterList(arr, "a", ["name"])); // => [{name: "a", job:"d"}]
arr2 = [{name: "a", job: "b", employees: [{name: "d", job:"e" }]}, {name: "f", job: "g"}];
console.log(filterList(arr2, "d", ["name", "employees"])); //=> [{name: "a", job:"b", employees: [{name: "d", job: "e"}]}]
