Sam van beastlo
Sam van beastlo

Reputation: 849

ugly nested forEach in javascript best practise

Hi my code feels very hacky, i could do the same with forloop which would be better for performance but it will look even more horrible. Is there a cleaner/best practise way of doing this?

this is my datamodel

this.data = [
  {
    title: 'Math',
    img: this.mathImage,
    children: [
      {
        title: 'Calculus I & II',
        open: false,
        children: [
          {
            title: 'Differentials',
            open: false,
            children: [
              {
                title: 'Integration by parts',
                key: 'Differentials1',
                mainsub: 'Math',
                middlesub: 'Calculus I & II',
                lowsub: 'Differentials',
                saved: true // <--------------- HERE IS THE PROPERTY
              },
              {
                title: 'Integration by parts',
                key: 'Differentials2',
                mainsub: 'Math',
                middlesub: 'Calculus I & II',
                lowsub: 'Differentials',
                saved: true,
              },
            ]
          }
        ]
      }
    ]
  }
]

this is my code that sets the 'saved' property to false

removeFromFavoritesinSubjects(item) {
  this.data.forEach(list => {
    if (list.title === item.mainsub) {
      list.children.forEach(subitem => {
        if (subitem.title === item.middlesub) {
          subitem.children.forEach(items => {
            if (items.title === item.lowsub) {
              items.children.forEach(i => {
                if (i.key === item.key) {
                  i.saved = false;
                }
              })
            }
          })
        }
      })
    }
  })
} 

Upvotes: 0

Views: 5827

Answers (3)

Madmadi
Madmadi

Reputation: 2140

You can take advantage of recursion and .reduce function to flatten your data model first and then do what should be done with your flat data model easily.

// flattenArray helper function turns nested array into flat array
function flattenArray (array) {
  return array.reduce((result, item) =>
    result.concat([ item ], flattenArray(item.children || []))
  , []);
}

// later in code
{
  // ...
  removeFromFavoritesInSubjects ({ key }) {
    // Consider all children are alongside with each other
    // [..., { title: 'Calculus I & II', ... }, { title: 'Differentials', ... }, ...]
    flattenArray(this.data).forEach(item => {
      if (item.key === key) {
        item.saved = false;
      }
    });
  }
  // ...
}

Also don't worry too much about performance to use for..loop against forEach.

Upvotes: 0

Graham
Graham

Reputation: 7802

I'm going to assume that you only want to change saved to false when you're at the lowest level (i.e. there are no saved properties at other levels).

This answer uses for... of to loop through the arrays recursively.

function changeSaved(data, key, value) {

  // loop through every object in the array
  for(var element in data){

    // see if there is a children node
    if(element.children){

      // run this function recursively on the children array
      changeSaved(element.children);

    } else {

      // no children key, set the value
      element[key] = value;
    }

  }

}

You would call it like this:

changeSaved(data, 'saved', 'false');

Upvotes: 2

Bergi
Bergi

Reputation: 664385

Searching a data structure linearly is very inefficient, if you have to do this more often you should either store a direct reference to the object in item or use a data structure that allows efficient lookup by key (e.g. a Map).

Instead of nested loops (that would be better with for … of than .forEach()) I would use chained .find() invocations though:

removeFromFavoritesinSubjects(item) {
  const list = this.data.find(list => list.title === item.mainsub);
  if (!list) return;
  const subitem = list.children.find(subitem => subitem.title === item.middlesub);
  if (!subitem) return;
  const items = subitem.children.find(items => items.title === item.lowsub);
  if (!items) return;
  const i = items.children.find(i => i.key === item.key);
  if (!i) return;
  i.saved = false;
}

(Assuming that there are no duplicates in the array, and each removeFromFavouritesInSubjects() call sets at most one .saved property).

Upvotes: 0

Related Questions