patternjavascriptMinor
Advice needed in refactoring/cleaning up my javascript code
Viewed 0 times
refactoringneededjavascriptadvicecodecleaning
Problem
I am fairly new to Javascript. I have written a website that works as designed. However, I have a strong feeling that my code could be heavily refactored. Can anyone see anything, besides line spacing, that I could do to clean up my code?
```
var i1 = document.createElement("img");
var i2 = document.createElement("img");
var i3 = document.createElement("img");
var i4 = document.createElement("img");
var i5 = document.createElement("img");
var i6 = document.createElement("img");
var i7 = document.createElement("img");
var i8 = document.createElement("img");
var i9 = document.createElement("img");
i1.type = "image";
i2.type = "image";
i3.type = "image";
i4.type = "image";
i5.type = "image";
i6.type = "image";
i7.type = "image";
i8.type = "image";
i9.type = "image";
var floatingButton1 = document.getElementById('floatingButton1');
var floatingButton2 = document.getElementById('floatingButton2');
var floatingButton3 = document.getElementById('floatingButton3');
var floatingButton4 = document.getElementById('floatingButton4');
var floatingButton5 = document.getElementById('floatingButton5');
var floatingButton6 = document.getElementById('floatingButton6');
var floatingButton7 = document.getElementById('floatingButton7');
var floatingButton8 = document.getElementById('floatingButton8');
var floatingButton9 = document.getElementById('floatingButton9');
floatingButton1.style.paddingLeft="35px";
floatingButton2.style.paddingLeft="35px";
floatingButton3.style.paddingLeft="35px";
floatingButton4.style.paddingLeft="35px";
floatingButton5.style.paddingLeft="35px";
floatingButton6.style.paddingLeft="35px";
floatingButton7.style.paddingLeft="35px";
floatingButton8.style.paddingLeft="35px";
floatingButton9.style.paddingLeft="35px";
floatingButton1.style.paddingTop="5px";
floatingButton2.style.paddingTop="5px";
floatingButton3.style.paddingTop="5px";
floatingButton4.style.paddingTop="5px";
floatingButton5.style.paddingTop="5px";
floatingButton6.style.paddingTop="5px";
floatingButto
```
var i1 = document.createElement("img");
var i2 = document.createElement("img");
var i3 = document.createElement("img");
var i4 = document.createElement("img");
var i5 = document.createElement("img");
var i6 = document.createElement("img");
var i7 = document.createElement("img");
var i8 = document.createElement("img");
var i9 = document.createElement("img");
i1.type = "image";
i2.type = "image";
i3.type = "image";
i4.type = "image";
i5.type = "image";
i6.type = "image";
i7.type = "image";
i8.type = "image";
i9.type = "image";
var floatingButton1 = document.getElementById('floatingButton1');
var floatingButton2 = document.getElementById('floatingButton2');
var floatingButton3 = document.getElementById('floatingButton3');
var floatingButton4 = document.getElementById('floatingButton4');
var floatingButton5 = document.getElementById('floatingButton5');
var floatingButton6 = document.getElementById('floatingButton6');
var floatingButton7 = document.getElementById('floatingButton7');
var floatingButton8 = document.getElementById('floatingButton8');
var floatingButton9 = document.getElementById('floatingButton9');
floatingButton1.style.paddingLeft="35px";
floatingButton2.style.paddingLeft="35px";
floatingButton3.style.paddingLeft="35px";
floatingButton4.style.paddingLeft="35px";
floatingButton5.style.paddingLeft="35px";
floatingButton6.style.paddingLeft="35px";
floatingButton7.style.paddingLeft="35px";
floatingButton8.style.paddingLeft="35px";
floatingButton9.style.paddingLeft="35px";
floatingButton1.style.paddingTop="5px";
floatingButton2.style.paddingTop="5px";
floatingButton3.style.paddingTop="5px";
floatingButton4.style.paddingTop="5px";
floatingButton5.style.paddingTop="5px";
floatingButton6.style.paddingTop="5px";
floatingButto
Solution
To add to what @Pimgd said, instead of using a
Whenever you see this pattern:
replace it with this:
As an aside, in Javascript unless you explicitly intend to create a global variable (which you nearly always should not), then you should use the
while loop in clearButtons and makeOptionButtons and you should use a for loop.Whenever you see this pattern:
var i = 1;
while (i < 10) {
// ...
i = i + 1;
}replace it with this:
for (var i = 1; i < 10; i++) {
// ...
}As an aside, in Javascript unless you explicitly intend to create a global variable (which you nearly always should not), then you should use the
var keyword. For example your i = 1; should be var i = 1;.Code Snippets
var i = 1;
while (i < 10) {
// ...
i = i + 1;
}for (var i = 1; i < 10; i++) {
// ...
}Context
StackExchange Code Review Q#57741, answer score: 8
Revisions (0)
No revisions yet.