#Keep it simple
Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the iteration features on it. EGdocument.querySelectorAll("query string").forEach(
and[...document.querySelectorAll("query string")]
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the iteration features on it. EGdocument.querySelectorAll("query string").forEach(
and[...document.querySelectorAll("query string")]
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the iteration features on it. EGdocument.querySelectorAll("query string").forEach(
and[...document.querySelectorAll("query string")]
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterableiterable object. There is no need to convert it to an array. You can usefor of
or any of the array iteration functionsfeatures on it. EGdocument.querySelectorAll("query string").forEach(
and[...document.querySelectorAll("query string")]
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the array iteration functions on it.document.querySelectorAll("query string").forEach
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the iteration features on it. EGdocument.querySelectorAll("query string").forEach(
and[...document.querySelectorAll("query string")]
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. TheThere is no need to convert it to an array. You can usefor of
or any of the array iteration functions on it.document.querySelectorAll("query string").forEach
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. The is no need to convert it to an array. You can usefor of
or any of the array iteration functions on it.document.querySelectorAll("query string").forEach
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();
#Keep it simple
Your algorithm is too complex, with too many iterations and uses too much memory. It can be simplified using only a single outer iteration to get a row, and the row's max height. Then an inner loop to set the new height values. (see rewrite)
Some points
document.querySelectorAll
returns an iterable object. There is no need to convert it to an array. You can usefor of
or any of the array iteration functions on it.document.querySelectorAll("query string").forEach
You only need to use
.getAttribute
when the attribute is not part of the DOM.const height = element.height
is more concise.You can use the
Math
functionmax
to get the maximum of 2 or more values at a time.maxHeight = Math.max(maxHeight, element.height)
Don't pollute the global scope. Use a self invoking function to encapsulate your variables.
;(()=>{ /* your code in here */ })();
##Rewrite
;(() => {
"use strict";
const itemsPerRow = 3;
const query = ".summary-image-wrapper";
const minHeight = 100; // Maybe a min height just in case
const row = [];
var maxHeight = minHeight;
document.querySelectorAll(query)
.forEach((imgWrap, i, imgWrappers) => {
row.push(imgWrap);
maxHeight = Math.max(imgWrap.firstElementChild.height , maxHeight);
if (row.length === itemsPerRow || i === imgWrappers.length - 1) {
row.forEach(imgWrap => imgWrap.style.height = `${maxHeight}px`);
maxHeight = minHeight;
row.length = 0;
}
});
})();