0

Inside a filter I need to test for a variety of conditions.

filter(books, (book) => {

    return myFilters.authors.map(a => a.id).includes(book.author_id) &&
    myFilters.skus.map(a => a.id).includes(book.sku);
    ....

There are other conditions depending on the type of filter, e.g. myFilters.price will be a different test.

I'm just wondering if you could point me in the right direction in terms of testing inside the filter, as if myFilter does not have any authors the map method will fail. How can I test for authors existing on myFilters (using something like authors in myFilters) but inside the filter method?

Something like:

filter(books, (book) => {

    return DOES AUTHORS EXIST ON MY FILTERS? myFilters.authors.map(a => a.id).includes(book.author_id) &&
    DO SKUS EXIST ON MY FILTERS? myFilters.skus.map(a => a.id).includes(book.sku);
    ....
panthro
  • 22,779
  • 66
  • 183
  • 324
  • on a side note you should not call your function filter, as it's already a native method in javascript. Now for your problem, you could chain methods, as in `myFilters.filter(filter => filter.hasOwnProperty('author')).map(etc..)` – Bernard Pagoaga Apr 05 '19 at 15:02
  • Possible duplicate of [Check if array is empty or does not exist. JS](https://stackoverflow.com/questions/24403732/check-if-array-is-empty-or-does-not-exist-js) – Liam Apr 05 '19 at 15:02
  • @BernardPagoaga that would not work as when a property does not exist, it will return false inside the entire filter, I want it to ignore it and not return false. – panthro Apr 05 '19 at 15:04

2 Answers2

2

You could use the logical || OR operator to substitute an empty array when one does not exist.

return (myFilters.authors || []).map(...)

But I think it's better if you're able to control the structure to make it such that there will always be an array, even if empty.


If you need to exclude that part of the filter when the array doesn't exist, then use the conditional operator.

return (myFilters.author ? myFilters.authors.map(...).includes(...) : true) &&
       (myFilters.skus ? myFilters.skus.map(...).includes(...) : true)

However, it really seems like a separate function would be of benefit here. Something like this:

function includesIfExists(arr, prop, val) {
  return arr ? arr.map(v => v[prop]).includes(val) : true
}

Then:

return includesIfExists(myFilters.authors, "id", book.id) && ...

And actually, the function could use .some() so that you're not iterating twice.

function includesIfExists(arr, prop, val) {
  return arr ? arr.some(v => v[prop] === val) : true
}
ziggy wiggy
  • 1,039
  • 6
  • 6
0
filter(books, (book) => {
    return myFilters.authors.map(a => a.id).includes(book.author_id) &&  
    myFilters.skus.map(a => a.id).includes(book.sku);  
    ....

You could easily check for the truthiness and perform your expression if it exists. This is known as a Guard. If it doesn't evaluate to true, using the OR operator will default it to true so your return expression can proceed. It's very similar to a ternary operator.

Consider:

filter(books, (book) => {
  return 
    ( myFilters.authors && myFilters.authors.map(a => a.id).includes(book.author_id) || true ) &&
    ( myFilters.skus && myFilters.skus.map(a => a.id).includes(book.sku) || true );
    ...

Readability

There are several reasons why you may not want to do this. The main reason may be for readability and maintainability. Your code is short and concise; however, 5 years from now (or even in 2 months) will you be able to quickly know how to modify it or fix a bug? Will someone else?

For that, it's generally better to be more expressive:

filter(books, (book) => {
   let has_authors     = true       // default case if author filters not supplied
   let has_skus        = true       // default case if skus filters not supplied
   let {authors, skus} = myFilters  // destructuring assignment for code legibility

   if ( authors )
     has_authors = authors.map(a => a.id).includes(book.author_id)

   if ( skus )
     has_skus = skus.map(a => a.id).includes(book.sku)

  return has_authors && has_skus
}

Of course, this could be further improved using some to minimize the number of iterations (one loop instead of two) and a function call to reduce the syntax, making it DRYer:

filter(books, (book) => {
   let has_authors     = true       // default case if author filters not supplied
   let has_skus        = true       // default case if skus filters not supplied
   let {authors, skus} = myFilters  // destructuring assignment for code legibility

   if ( authors )
     has_authors = hasId( authors, book.author_id )

   if ( skus )
     has_skus = hasId( skus, book.sku )

  return has_authors && has_skus
}

function hasId( array, id ) {
  return array.some(v => v.id == id )
}

Working Example of the Above

setup() // create globals and functions


let filtered_books = filter(books, (book) => {
   let has_authors     = true       // default case if author filters not supplied
   let has_skus        = true       // default case if skus filters not supplied
   let {authors, skus} = myFilters  // destructuring assignment for code legibility

   if ( authors )
     has_authors = hasId( authors, book.author_id )

   if ( skus )
     has_skus = hasId( skus, book.sku )

  return has_authors && has_skus
})

function hasId( array, id ) {
  return array.some(v => v.id == id )
}

console.log(`filtered_books: `, filtered_books)
console.log(`from: `, books)





function setup(){

  window.myFilters = {
    // authors: [ { id: 7 } ],  // <-- uncomment to show a case where no authors match
    skus: [
      { id: 3 },
      { id: 5 }
    ]
  }
  window.books = [
    {author_id:4, sku: 2},
    {author_id:5, sku: 3}
  ]
  window.filter = function(array, callback){
    return array.filter(callback)
  }
}
Mike
  • 1,279
  • 7
  • 18