HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Advice needed in refactoring/cleaning up my javascript code

Submitted by: @import:stackexchange-codereview··
0
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

Solution

To add to what @Pimgd said, instead of using a 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.