1
\$\begingroup\$

I am looking for best performance regarding this code point is I don't have a clear idea weather a map integrated in a for loop is a good idea, the problem is the array is a nested object coming from Db that I don't have any control of, could there be a better version ? the code does the following the for loop populates data to divs using info in the Incoming array those info varies for example one day will have 3 element other only one the first div in a row should get the data from the first object in the incoming arr the second will get the second etc..>

let [...divMulti] = document.querySelectorAll('.insert')
 
let Incoming = [{ day: [{ el: '1' }, { el: '2' }, { el: '3' }] }, { day: [{ el: '1' }] }, { day: [{ el: '1' }] }]
        let data1 = Incoming.map(element => {
            return element.day
        })
        for (let i = 0; i < divMulti.length; i++) {
            data1[i].map(el => {
                let item = `<div>Element${i}</div>`
                divMulti[i].querySelector('.here').insertAdjacentHTML('afterbegin', item)
            })
        }
<div class='insert' >parentDiv
<div class='here'></div>
</div>


<div class='insert' >parentDiv
<div class='here'></div>
</div>

<div class='insert' >parentDiv
<div class='here'></div>
</div>

\$\endgroup\$
1
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Jun 16, 2021 at 6:42

1 Answer 1

1
\$\begingroup\$

array.map() is meant to convey that you're transforming the values of one array into another in a different array. If you're intending to do a side-effect (like printing to the console, or manipulating the DOM) iteratively and not return a result, array.forEach() conveys this intention better.

There's also nothing wrong with a nested loop either. If you feel like it's much more readable to write loops than other methods, then go for it. Just make sure to be consistent and name your variables in a way that's not confusing.

Unless you're dealing with hundreds of thousands of data, or needing to render this in 60fps, I wouldn't worry so much about performance. What I would like you to worry about is code readability, and consistency.

In situations like these, I often prefer flattening the data so that each step is just an operation on an array (i get lost in nested loops). At any point I want to debug the result in the chain, I can just pop the result out into a variable for inspection. This is just my way of keeping the code readable, yours may be different.

let [...divMulti] = document.querySelectorAll('.insert')

let Incoming = [
  { day: [{el: '1'}, {el: '2'}, {el: '3'}] },
  { day: [{el: '4'}] }, 
  { day: [{ el: '5' }] }
]

// Just a utility function you find in other languages. Creates an
// array of pairs from two arrays.
const zip = (a, b) => a.map((k, i) => [k, b[i]]);

// Create insert-incoming pairs.
// [
//   [insert[0], incoming[0]],
//   [insert[1], incoming[1]],
//   [insert[2], incoming[2]],
// ]
zip(divMulti, Incoming)
  // This step then returns here-day.el pairs
  // [
  //   [here[0], day[0].el],
  //   [here[0], day[1].el],
  //   [here[0], day[2].el],
  //   [here[1], day[0].el],
  //   [here[2], day[0].el],
  // ]
  .flatMap(([div, incomingItem]) => {
    const here = div.querySelector('.here')
    return incomingItem.day.map(dayItem => [here, dayItem.el])
  })
  // foreach to convey that we're just looping, not expecting a return.
  // insert each el value to its corresponding here.
  .forEach(([here, el]) => {
    here.insertAdjacentHTML('afterbegin', `<div class="item">Element${el}</div>`)
  })
.insert { border: 1px solid red; padding: 5px }
.here { border: 1px solid green; padding: 5px }
.item { border: 1px solid blue; padding: 5px }
<div class='insert'>parentDiv
  <div class='here'></div>
</div>


<div class='insert'>parentDiv
  <div class='here'></div>
</div>

<div class='insert'>parentDiv
  <div class='here'></div>
</div>

\$\endgroup\$
1
  • \$\begingroup\$ thanks for the time and the great answer. \$\endgroup\$ Jun 16, 2021 at 5:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.