patternjavascriptMinor
Mouseover effects for five buttons which may be enabled or disabled
Viewed 0 times
enabledmayfiveeffectsdisabledbuttonsmouseoverforwhich
Problem
```
function prepareEventHandlers() {
var sectionButton1 = document.getElementById("sectionButton1");
var sectionButton2 = document.getElementById("sectionButton2");
var sectionButton3 = document.getElementById("sectionButton3");
var sectionButton4 = document.getElementById("sectionButton4");
var sectionButton5 = document.getElementById("sectionButton5");
var enabled1 = true;
var enabled2 = false;
var enabled3 = false;
var enabled4 = false;
var enabled5 = false;
function checkEnabled() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonEnabled");
}
if (enabled3) {
sectionButton3.setAttribute("class", "sectionButtonEnabled");
}
if (enabled4) {
sectionButton4.setAttribute("class", "sectionButtonEnabled");
}
if (enabled5) {
sectionButton5.setAttribute("class", "sectionButtonEnabled");
}
}
checkEnabled();
sectionButton1.onmouseover = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonOver");
}
};
sectionButton1.onmouseout = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
};
sectionButton2.onmouseover = function() {
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonOver");
}
};
sectionButton2.onmouseout = function() {
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonEnabled");
}
};
sectionButton3.onmouseover = function() {
if (enabled3) {
sectionButton3.setAttribute("class", "sectionButtonOver");
}
};
sectionButton3.onmouseout = function() {
if (enabled3) {
sectionButton3.set
function prepareEventHandlers() {
var sectionButton1 = document.getElementById("sectionButton1");
var sectionButton2 = document.getElementById("sectionButton2");
var sectionButton3 = document.getElementById("sectionButton3");
var sectionButton4 = document.getElementById("sectionButton4");
var sectionButton5 = document.getElementById("sectionButton5");
var enabled1 = true;
var enabled2 = false;
var enabled3 = false;
var enabled4 = false;
var enabled5 = false;
function checkEnabled() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonEnabled");
}
if (enabled3) {
sectionButton3.setAttribute("class", "sectionButtonEnabled");
}
if (enabled4) {
sectionButton4.setAttribute("class", "sectionButtonEnabled");
}
if (enabled5) {
sectionButton5.setAttribute("class", "sectionButtonEnabled");
}
}
checkEnabled();
sectionButton1.onmouseover = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonOver");
}
};
sectionButton1.onmouseout = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
};
sectionButton2.onmouseover = function() {
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonOver");
}
};
sectionButton2.onmouseout = function() {
if (enabled2) {
sectionButton2.setAttribute("class", "sectionButtonEnabled");
}
};
sectionButton3.onmouseover = function() {
if (enabled3) {
sectionButton3.setAttribute("class", "sectionButtonOver");
}
};
sectionButton3.onmouseout = function() {
if (enabled3) {
sectionButton3.set
Solution
Well first you should think about refactoring your code. The following
is repeated a few times with just 2 differences. The enabled flag and the button. So this could easily go into a new function
The following is also repeated:
so this could easily go into another function as well. But I would change one thing. You are adding the mouse events to every button but they only do something if they are enabled. So I would check that first and then add the mouse events.
Doing just the above will shorten your code quite a bit because you can then call the functions like this
But I took it a step farther. By putting the buttons and the enabled flags into an array you can just iterate over each and do the work if necessary. Doing it this way will make it extremely easy to add another button in the future. The following code can replace all the above mentioned changes.
I would actually take the array out of this function and pass it in as an argument but you get the point.
I'm also assuming the enabled flags may change for any of the buttons. If not and it is only one (as you are showing) you could just eliminate all the code that has nothing to do with button1 because none of the others are enabled so they are not manipulated at all.
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}is repeated a few times with just 2 differences. The enabled flag and the button. So this could easily go into a new function
function SetClassIfEnabled(enabled, btn) {
if (enabled) {
btn.setAttribute("class", "sectionButtonEnabled");
}
}The following is also repeated:
sectionButton1.onmouseover = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonOver");
}
};
sectionButton1.onmouseout = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
};so this could easily go into another function as well. But I would change one thing. You are adding the mouse events to every button but they only do something if they are enabled. So I would check that first and then add the mouse events.
function AddHoverIfEnabled(enabled, btn) {
if (enabled) {
btn.onmouseover = function () {
btn.setAttribute("class", "sectionButtonOver");
};
btn.onmouseout = function () {
btn.setAttribute("class", "sectionButtonEnabled");
};
}
}Doing just the above will shorten your code quite a bit because you can then call the functions like this
SetClassIfEnabled(enabled1, sectionButton1);
SetClassIfEnabled(enabled2, sectionButton2);
...
AddHoverIfEnabled(enabled1, sectionButton1);
AddHoverIfEnabled(enabled2, sectionButton2);
...But I took it a step farther. By putting the buttons and the enabled flags into an array you can just iterate over each and do the work if necessary. Doing it this way will make it extremely easy to add another button in the future. The following code can replace all the above mentioned changes.
function prepareEventHandlers() {
var btnArray = [];
btnArray[0] = [document.getElementById("sectionButton1"), true];
btnArray[1] = [document.getElementById("sectionButton2"), false];
btnArray[2] = [document.getElementById("sectionButton3"), false];
btnArray[3] = [document.getElementById("sectionButton4"), false];
btnArray[4] = [document.getElementById("sectionButton5"), false];
for (i = 0; i < btnArray.length; i++) {
if (btnArray[i][1] == true) { // section/button is enabled
var btn = btnArray[i][0];
btn.setAttribute("class", "sectionButtonEnabled"); // set default class
// Add Hover Events
btn.onmouseover = function () {
this.setAttribute("class", "sectionButtonOver");
};
btn.onmouseout = function () {
this.setAttribute("class", "sectionButtonEnabled");
};
}
}
}I would actually take the array out of this function and pass it in as an argument but you get the point.
I'm also assuming the enabled flags may change for any of the buttons. If not and it is only one (as you are showing) you could just eliminate all the code that has nothing to do with button1 because none of the others are enabled so they are not manipulated at all.
Code Snippets
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}function SetClassIfEnabled(enabled, btn) {
if (enabled) {
btn.setAttribute("class", "sectionButtonEnabled");
}
}sectionButton1.onmouseover = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonOver");
}
};
sectionButton1.onmouseout = function() {
if (enabled1) {
sectionButton1.setAttribute("class", "sectionButtonEnabled");
}
};function AddHoverIfEnabled(enabled, btn) {
if (enabled) {
btn.onmouseover = function () {
btn.setAttribute("class", "sectionButtonOver");
};
btn.onmouseout = function () {
btn.setAttribute("class", "sectionButtonEnabled");
};
}
}SetClassIfEnabled(enabled1, sectionButton1);
SetClassIfEnabled(enabled2, sectionButton2);
...
AddHoverIfEnabled(enabled1, sectionButton1);
AddHoverIfEnabled(enabled2, sectionButton2);
...Context
StackExchange Code Review Q#6487, answer score: 7
Revisions (0)
No revisions yet.