Suggestions for optimising this front end code

Question

I've stumbled across some UI (React.js) code that I've worked out to have a time complexity of O(n^4) (I hope that's right, if not please do correct me). I was interested in how one might go about optimising something like this.

I know that for() loops do perform slightly better than array.forEach() and I'd assume const uses a bit less memory than let due to not needing to support re-allocation.

Would using functional operators like map and filter improve performance?

I'd be really interested to see some suggestions to this, and maybe the ideal optimisation for it?

The code:

animate () {
 let movingScreens = {}
 if (this.animationsDisabled()) {
 movingScreens = {}
 } else {
 for (let i in scrollAnimationDefs.movingScreens) {
 if (i !== 'appsTop' && i !== 'appsBottom') movingScreens[i] = scrollAnimationDefs.movingScreens[i]
 }
 }
 for (let shape in movingScreens) {
 if (movingScreens[shape].scrolls) {
 for (let scroll of movingScreens[shape].scrolls) {
 const from = scroll.from * this.scrollMax
 const to = scroll.to * this.scrollMax
 if (from <= this.scrollTop && to >= this.scrollTop) {
 const styles = scroll.styles(
 (this.scrollTop - from) / (to - from)
 )
 for (let style in styles) {
 let newStyle = styles[style]
 const els = scrollAnimationDefs.movingScreens[shape].els.map(
 ref => {
 if (ref === 'screen1IfContainer') return this.animations.screen1IfContainer.current.container.current
 return this.animations[ref].current
 }
 )
 for (const i in els) {
 if (els[i]) {
 els[i].style[style] = newStyle
 }
 }
 }
 }
 }
 }
 }
}

• 200_success: Please tell us, what does this code do? See How to Ask.
• BCdotWEB: Your title is too generic. It should reflect what your code does, not what you want out of the review.

Answer 1

Style and code

• There are several places where you should use const rather than let. Mainly inside the for loops.
• The second movingScreens = {} is redundant.
• Avoid nesting loops too deeply. Use a function to avoid this.
• Don't use for...in as you need to be sure you are not getting properties higher up the prototype chain. Use Object.keys to get the keys and iterate them with a for...of
• Avoid long lines by using alias to reference object paths or long names.
• The naming is confusing, you have shapes, screens, scrollScreens that seam to be interchangeable
• Is this a typo or a really bad name and reference this.animations.screen1IfContainer.current.container.current

Avoid iterating again

You build the object movingScreens then you iterate it again. Would be better to process the items instead of building the object movingScreens.

You use Array.map to create an array of elements, then you iterate that array again. Process the elements as you iterate the first time.

Rewrite

• Removed the object movingScreens processing the shapes in the first loop.
• Added function scrollScreen to process a (screen, shape, or scroll I can not be sure).
• Used aliases to reference long names and paths. currentCont, and screens
• All variables are now const
• Used Object.keys for safer key iteration.

As there is no way for me to test this code, nor do I have a context (what is what), it may contain typos or logic errors.

animate() {
 const currentCont = this.animations.screen1IfContainer.current.container.current;
 const scrollScreen = shape => {
 for (const scroll of shape.scrolls) {
 const from = scroll.from * this.scrollMax;
 const to = scroll.to * this.scrollMax;
 if (from <= this.scrollTop && to >= this.scrollTop) {
 const styles = scroll.styles((this.scrollTop - from) / (to - from));
 for (const style of Object.keys(styles)) {
 for(const ref of shape.els) {
 const el = ref === 'screen1IfContainer' ? 
 currentCont : this.animations[ref].current;
 if (el) { el.style[style] = styles[style] }
 }
 }
 }
 }
 }
 if (!this.animationsDisabled()) {
 const screens = scrollAnimationDefs.movingScreens;
 for (const key of Object.keys(screens)) {
 if (key !== 'appsTop' && key !== 'appsBottom' && screens[key].scrolls) {
 scrollScreen(screens[key]);
 }
 }
 }
}

Questions

\$O(n^4)\$ (I hope that's right, if not please do correct me)

You have not defined n thus its is meaningless to define the complexity.

Would using functional operators like map and filter improve performance? I'd be really interested to see some suggestions to this, and maybe the ideal optimisation for it?

As this is animation these points can not be answered. The slow point is the moving of pixels, this is highly dependent on the device it is running on. The JS code is but a fraction of the workload, you can optimize it till the cows come home and it may not make a spec of difference on the performance of the animation.

Any optimization would require the full animation to see when and where the many GPU state changes, compositing, and what not can be optimized.

Update

I forgot this line in your question.

I'd assume const uses a bit less memory than let due to not needing to support re-allocation

If const uses less memory that would be up to the JS engine. I would not use it as a criteria as when to use const or not.

• We use const to protect against accidental mutation.
• We use let and const to avoid accidentally redefining a variable.
• We use const (as with let) to scope the variable to a block. A block is code between { and } The exception is when a variable is defined in a for, do, or while loop as a const or let it is scoped to the loop block, not to the current block. eg { /*current block a is not in scope */ for(const a of b) { /* a is in scope loop block*/ } }
• We use const to give meaning to constants that are otherwise just numbers or strings in your source code. eg const unit = 1;

• Currently chrome, FF, edge give const, let, and var have a slight performance benefit over literals. eg 1 + 1 is a little slower than one + one with one defined as const one = 1;

  Do not rely on this to remain true as engines are constantly changing. The benefit is very small and only an advantage under very specific conditions.

• Sᴀᴍ Onᴇᴌᴀ: "There are several places where you should use const rather than let. Mainly inside the for loops" I know the presumed reason but the OP might not. You should explain the advantages (and possible disadvantages as well).
• Blindman67: @SᴀᴍOnᴇᴌᴀ Was going to address OP's comment about const and forgot. Added some more words on the matter.