C7f0f836bc69eeeaa3ab061ac2b6d8d7

I am pretty new to javascript, i wrote a javascript which changes a picture in html if the user goes over the names of list (identified by id). Any suggestion to refactor the code?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
var Fenster = null;
function neuesFenster(meineSeite,meinName,w,h,scroll){
LeftPosition = (screen.width) ? (screen.width-w)/2 : 0;
TopPosition = (screen.height) ? (screen.height-h)/2 : 0;
settings =
'height='+h+',width='+w+',top='+TopPosition+',left='+LeftPosition+',scrollbars='+scroll+',resizable'
Fenster = window.open(meineSeite,meinName,settings)
}


function AddEventListener(element, eventType, handler, capture)
{
	if (element.addEventListener)
		element.addEventListener(eventType, handler, capture);
	else if (element.attachEvent)
		element.attachEvent("on" + eventType, handler);
}

window.onload = function()
{
	AddEventListener(document.getElementById("mr"), "mouseover", function(e)
	{
		zeigbild('mr');
	}, 

	AddEventListener(document.getElementById("ib"), "mouseover", function(e)
	{
		zeigbild('ib');
		},
	AddEventListener(document.getElementById("fr"), "mouseover", function(e)
	{
		zeigbild('fr');
	},
	AddEventListener(document.getElementById("nh"), "mouseover", function(e)
	{
		zeigbild('nh');
	},
	AddEventListener(document.getElementById("fz"), "mouseover", function(e)
	{
		zeigbild('fz');
	},
	AddEventListener(document.getElementById("mm"), "mouseover", function(e)
	{
		zeigbild('mar');
	},
	AddEventListener(document.getElementById("je"), "mouseover", function(e)
	{
		zeigbild('je');
	},
	AddEventListener(document.getElementById("db"), "mouseover", function(e)
	{
		zeigbild('db');
	},
	AddEventListener(document.getElementById("ms"), "mouseover", function(e)
	{
		zeigbild('ms');
	},false)))))))));
};


function zeigbild(q){ 
document.getElementById('foto').setAttribute('src','0'+q+'.jpg')
}

Refactorings

No refactoring yet !

F63942474f436f8a663a76ee23164965

naholyr

November 6, 2007, November 06, 2007 16:32, permalink

2 ratings. Login to rate!

I would use a Hash to reduce code and ease maintainability
I also use AddEventListener instead of window.onload : if you want to add more events to your window.onload, you can this way.

Here is my refactoring for the code after declaration of AddEventListener

Note that you could have more gains using prototype.js

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
var thumbnailsToImage = {
  'mr' : '0mr.jpg',
  'ib' : '0ib.jpg',
  'fr' : '0fr.jpg',
  'nh' : '0nh.jpg',
  'fz' : '0fz.jpg',
  'mar': '0mar.jpg',
  'je' : '0je.jpg',
  'db' : '0db.jpg',
  'ms' : '0ms.jpg',
  'mr' : '0mr.jpg',
  'mr' : '0mr.jpg',
  'mr' : '0mr.jpg',
};

AddEventListener(window, "load", function() {
  for (thumbnailID in thumbnailsToImage) {
    AddEventListener(document.getElementById(thumbnailID), 'mouseover', function() {
      zeigbild(thumbnailID);
    });
  }
});

function zeigbild(thumbnailID){ 
  document.getElementById('foto').setAttribute('src', thumbnailsToImage[thumbnailID]);
}
0938cdc250608469388f1645e4eabc5c

Rob Womack

November 7, 2007, November 07, 2007 14:42, permalink

No rating. Login to rate!

Since you have a naming convention set for your rollovers, you don't need to set a hash, or even to identify them by name. Give all your elements a common class name, like 'zeigbild-over', then call getElementsByClassName and assign the onmouseover event to each element returned. This way, to add a new element, just give it the appropriate class name and make sure the correctly named img file is uploaded.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
// getElementsByClassName borrowed from Stephen Chapman 
// (http://javascript.about.com/library/bldom08.htm)
document.getElementsByClassName = function(cl) { 
    var retnode = [];
    var myclass = new RegExp('\\b'+cl+'\\b');
    var elem = this.getElementsByTagName('*');
    for (var i = 0; i < elem.length; i++) {
        var classes = elem[i].className;
        if (myclass.test(classes)) retnode.push(elem[i]);
    }
    return retnode;
}; 

AddEventListener(window, "load", function() {
    var elements = document.getElementsByClassName('a');
    for (el in elements) {
        elements[el].onmouseover = function() { 
            document.getElementById('foto').setAttribute('src', '0' + this.id + 'jpg');
        };
    }
});

Your refactoring





Format Copy from initial code

or Cancel