Wolfgang Wohlwend
Wolfgang Wohlwend

Reputation: 23

Code review: Deep filter array of objects concisely in Javascript

I've been trying to wrap my head around filtering arrays of objects for a while now, but I can't seem to really get a hang on it. Although I usually have working code in the end, it just doesn't look like elegant code to me. So, I'd appreciate a code review and some hints very much!

Example: I'm currently working on this example for an online shop where I need to retrieve product details out of an array of objects based on an id.

This is my helper function:

function getItemDetails(id) {
        var getCategory = shelf.filter(obj => obj.articleList.some(cat => cat.id === id));
        var getArticleList = getCategory[0].articleList;
        var getItem = getArticleList.filter(item => item.id == id);
        return getItem 
}

Steps: In a first step, I tried to filter the shelf array, but it would return the entire array articleList of the corresponding item.

So, I filtered the result again with the same criteria and it works, but it just looks awfully redundant to me.

This is an example of the data:

const shelf = [{
    "categoryPrice": "2",
    "categoryTitle": "Flyer",
    "articleList": [{
        "id": "1",
        "articleTitle": "Green",
    }, {
        "id": "2",
        "articleTitle": "Blue",
    }],
}, {
    "categoryPrice": "3",
    "categoryTitle": "Post card",
    "articleList": [{
        "id": "3",
        "articleTitle": "Purple"
    }, {
        "id": "4",
        "articleTitle": "Yellow",
    }]
}]

I checked various questions here, including:

But none of them provide an easier, more concise solution, in my opinion. What am I missing?

Thanks for your help!

Upvotes: 2

Views: 875

Answers (3)

Akxe
Akxe

Reputation: 11575

This code is O(1), which means that lookup per article.id is constant. It will however use more memory. To conserve memory, I used WeakMap, as long as you use the same shelf variable, it will not recompute it. But once you replace it, it will also perish from the cache.

const shelf = [{
  "categoryPrice": "2",
  "categoryTitle": "Flyer",
  "articleList": [{
    "id": "1",
    "articleTitle": "Green",
  }, {
    "id": "2",
    "articleTitle": "Blue",
  }, {
    "id": "3",  //  Added
    "articleTitle": "Violet",
  }],
}, {
  "categoryPrice": "3",
  "categoryTitle": "Post card",
  "articleList": [{
    "id": "3",
    "articleTitle": "Purple"
  }, {
    "id": "4",
    "articleTitle": "Yellow",
  }],
}];

const findItems = function(shelves, id) {
  if (!findItems._map) {
    // Create computation cache holder
    // Weak map will make sure, that if the object is disposed, it can be garbage collected, with it will be gone its cache too! (That is awsome!)
    findItems._map = new WeakMap();
  }
  if (!findItems._map.has(shelves)) {
    // For every shelves object, we will create a new Map containing all mapped values.
    const map = new Map();
    findItems._map.set(shelves, map);
    shelves.forEach(shelf => {
      shelf.articleList.forEach(article => {
        if (!map.has(article.id)) {
          // If list is not yet created create it with the article
          return map.set(article.id, [ article ]);
        }
        
        // If it exists, add to it
        map.get(article.id).push(article);
      });
    });
  }

  return findItems._map.get(shelves).get(id);
}

console.log(findItems(shelf, "1"));
console.log(findItems(shelf, "3"));

Upvotes: 3

Jonathan Irwin
Jonathan Irwin

Reputation: 5767

You can get away with looping twice. Once for the outer array and once for the articleList array

const findItem = (id) =>
    shelf.reduce((acc, current) => {
        const found = current.articleList.find((x) => x.id === id);
        if (found) return [...acc, found];
        return acc;
    }, []);

Upvotes: 0

Jim Nilsson
Jim Nilsson

Reputation: 882

This might be a better fit for codereview but if the question is just 'How to make this more concise' I would suggest something like following:

const shelf = [{
  "categoryPrice": "2",
  "categoryTitle": "Flyer",
  "articleList": [{
    "id": "1",
    "articleTitle": "Green",
  }, {
    "id": "2",
    "articleTitle": "Blue",
  }],
}, {
  "categoryPrice": "3",
  "categoryTitle": "Post card",
  "articleList": [{
    "id": "3",
    "articleTitle": "Purple"
  }, {
    "id": "4",
    "articleTitle": "Yellow",
  }]
}]

const findItem = function(shelves, id) {
  return shelves.flatMap((shelf) => shelf.articleList).find((article) => article.id == id) || null;
}

console.log(findItem(shelf, 1));
console.log(findItem(shelf, 3));

The above example concatenate all the list of articles and then searches that array for the article with the supplied ID.

Performance wise? Not the best, but you asked for something concise and this is about as concise as one can hope for with the given data structure.

Upvotes: 4

Related Questions