patternjavascriptMinor
Foreach loop displaying instruction relative to a release number
Viewed 0 times
numberloopinstructiondisplayingreleaseforeachrelative
Problem
I've got a foreach loop, going through each section of an XML document (an example of said XML document can be found in this SO question) and only display them in divs with corresponding classes.
What I do after that is using a drop-down menu, and with the help of javascript switch the style.display property of the divs accordingly.
Form:
Javascript:
I've already refactored my javascript code, but there is still plenty of repetitive code out there, and it can only grow the more releases there will be.
EDIT: One more thing, those instruction are shown in a CSS pop-up using a t
What I do after that is using a drop-down menu, and with the help of javascript switch the style.display property of the divs accordingly.
Form:
10r3
11r2
12r1
12r2
13r1
Javascript:
function showHide(selection) {
var f = selection.form;
// Retrieve the value of the option selected.
var opt = selection.options[selection.selectedIndex].value;
/* Retrieve all the div elements of the document and then parse them to
* see if they have a classname corresponding with the selected release.
* If it is the case, remove all display style, otherwise set the
* display style to none.
*/
var divArray = document.getElementsByTagName("div");
for(var i=0; i<divArray.length; i++){
var relArray = ["10r3","11r2","12r1","12r2","13r1"];
for(var j=0; j<relArray.length; j++){
var relName = relArray[j];
if(divArray[i].className == "rel"+relName){
if(opt===relName){
divArray[i].style.display = "";
}else{
divArray[i].style.display = "none";
}
}
}
}
return false;
}I've already refactored my javascript code, but there is still plenty of repetitive code out there, and it can only grow the more releases there will be.
EDIT: One more thing, those instruction are shown in a CSS pop-up using a t
Solution
-
in your
-
Don't be skimpy on the formatting as it makes a big difference on the readability. forexample:
-
Javascript is very nice with falsey values:
can be written:
or
-
patterns like:
can be written much easier and cleaner:
-
you have:
can be:
-
you are using variables to hold a value for one line:
instead:
TL;DR? http://jsfiddle.net/37Uge/1/
in your
showHide() function:var relArray = ["rel10r3","rel11r2","rel12r1","rel12r2","rel13r1"];
for(var i=0; i<divArray.length; i++)
{
var currentDiv = divArray[i];
if(currentDiv.className && relArray.indexOf(currentDiv.className) != -1)
{
currentDiv.style.display = "";
}
else
{
currentDiv.style.display = "none";
}
}-
Don't be skimpy on the formatting as it makes a big difference on the readability. forexample:
function toggle(div_id) {
var el = document.getElementById(div_id);
if (el.style.display == 'none' )
{
el.style.display = 'block';
}
else
{
el.style.display = 'none';
}
}-
Javascript is very nice with falsey values:
if (typeof window.innerWidth != 'undefined') {
viewportheight = window.innerHeight;
} else {
viewportheight = document.documentElement.clientHeight;
}can be written:
if (window.innerWidth) {
viewportheight = window.innerHeight;
} else {
viewportheight = document.documentElement.clientHeight;
}or
viewportheight = window.innerHeight || document.documentElement.clientHeight;-
patterns like:
if(){
//
}else{
if(){
//
}else{
//
}
}can be written much easier and cleaner:
if(){
//
}else if(){
//
}else{
//
}-
you have:
if ((viewportwidth > document.body.parentNode.scrollWidth) && (viewportwidth > document.body.parentNode.clientWidth)) {
//etc etccan be:
window_width = Math.max(viewportwidth, document.body.parentNode.clientWidth, document.body.parentNode.scrollWidth);-
you are using variables to hold a value for one line:
var popUpDiv = document.getElementById(popUpDivVar);
window_width=50;
popUpDiv.style.left = window_width + 'px';instead:
var popUpDiv = document.getElementById(popUpDivVar);
if(popUpDiv) // incase it couldn't find the div.
{
popUpDiv.style.left = "50px";
}TL;DR? http://jsfiddle.net/37Uge/1/
Code Snippets
var relArray = ["rel10r3","rel11r2","rel12r1","rel12r2","rel13r1"];
for(var i=0; i<divArray.length; i++)
{
var currentDiv = divArray[i];
if(currentDiv.className && relArray.indexOf(currentDiv.className) != -1)
{
currentDiv.style.display = "";
}
else
{
currentDiv.style.display = "none";
}
}function toggle(div_id) {
var el = document.getElementById(div_id);
if (el.style.display == 'none' )
{
el.style.display = 'block';
}
else
{
el.style.display = 'none';
}
}if (typeof window.innerWidth != 'undefined') {
viewportheight = window.innerHeight;
} else {
viewportheight = document.documentElement.clientHeight;
}if (window.innerWidth) {
viewportheight = window.innerHeight;
} else {
viewportheight = document.documentElement.clientHeight;
}viewportheight = window.innerHeight || document.documentElement.clientHeight;Context
StackExchange Code Review Q#4861, answer score: 2
Revisions (0)
No revisions yet.