CODEWITHSUNDEEP
javascript
iceteea
iceteea

Reputation: 1224

Too much recursion

can anyone tell me where the loop comes from?

JS:

if (zahl > 1) {
    document.getElementById('makroclickm2').innerHTML = data_split[zahlm2];
    document.getElementById('makroclickm2').onclick = getwords(zahlm2++);
}
else {
    document.getElementById('makroclickm2').innerHTML = "";
    document.getElementById('makroclickm2').onclick = "";
}

if (zahl > 0) {
    document.getElementById('makroclickm1').innerHTML = data_split[zahlm1];
    document.getElementById('makroclickm1').onclick = getwords(zahlm1++);
}
else {
    document.getElementById('makroclickm1').innerHTML = "";
    document.getElementById('makroclickm1').onclick = "";
}

document.getElementById('makroclick').innerHTML = data_split[zahl];
document.getElementById('makroclick').onclick = getwords(zahl++);

document.getElementById('makroclickp1').innerHTML = data_split[zahlp1];
document.getElementById('makroclickp1').onclick = getwords(zahlp1++);
if (typeof(data_split[zahlp1]) == "undefined") {
    document.getElementById('makroclickp1').innerHTML = "";
    document.getElementById('makroclickp1').onclick = "";
}

document.getElementById('makroclickp2').innerHTML = data_split[zahlp2];
document.getElementById('makroclickp2').onclick = getwords(zahlp2++);
if (typeof(data_split[zahlp2]) == "undefined") {
    document.getElementById('makroclickp2').innerHTML = "";
    document.getElementById('makroclickp2').onclick = "";
}

HTML:

<div id="makroclickm2" onclick="" class="makroclick"></div>
<div id="makroclickm1" onclick="" class="makroclick"></div>
<div id="makroclick" onclick="getwords(0);" class="makroclick_center"></div>
<div id="makroclickp1" onclick="getwords(1);" class="makroclick"></div>
<div id="makroclickp2" onclick="getwords(2);" class="makroclick"></div>

(Not complete Code) The function is called once onload.

Thx in advance!

Upvotes: 0

Views: 937

Answers (4)

user166390
user166390

Reputation:

This doesn't answer the question (or even try to). However, it works to address the over-abundance of repeated code.

// instead of repeating the document.getElementById
// many times over...
document.getElementById('makroclickm2').innerHTML = "";
document.getElementById('makroclickm2').onclick = "";

// get the element once
var elm = document.getElementById('makroclickm2')
// and use it many times over
elm.innerHTML = ""
elm.onclick = null // do not use strings here

This will make the code much easier to follow (please use appropriate variable names).

Also, do not use strings with the onclick attribute. Use functions instead.

Happy coding.

Upvotes: 3

Blender
Blender

Reputation: 298106

This part might give you a ton of problems:

if (zahl > 1) {
    document.getElementById('makroclickm2').innerHTML = data_split[zahlm2];
    document.getElementById('makroclickm2').onclick = getwords(zahlm2++);
}
else {
    document.getElementById('makroclickm2').innerHTML = "";
    document.getElementById('makroclickm2').onclick = "";
}

if (zahl > 0) {
    document.getElementById('makroclickm1').innerHTML = data_split[zahlm1];
    document.getElementById('makroclickm1').onclick = getwords(zahlm1++);
}
else {
    document.getElementById('makroclickm1').innerHTML = "";
    document.getElementById('makroclickm1').onclick = "";
}

Try using else if instead of another block of if statements. Also, I'd use function(){} instead of "" (and getwords(), as that evaluates the function and doesn't set it to be run when the even is called):

if (zahl > 1) {
    document.getElementById('makroclickm2').innerHTML = data_split[zahlm2];
    document.getElementById('makroclickm2').onclick = function(){getwords(zahlm2++)};
} else if (zahl > 0) {
    document.getElementById('makroclickm1').innerHTML = "";
    document.getElementById('makroclickm1').onclick = null;

    document.getElementById('makroclickm1').innerHTML = data_split[zahlm1];
    document.getElementById('makroclickm1').onclick = function(){getwords(zahlm1++)};
} else {
    document.getElementById('makroclickm2').innerHTML = "";
    document.getElementById('makroclickm2').onclick = null;
}

Upvotes: 1

chprpipr
chprpipr

Reputation: 2039

You also might consider using event registration rather than direct assignment. That way you don't have to worry about accidentally overwriting anything later down the road. @Alnitak's solution would look more like this:

var myEl = document.getElementById('makroclickm2');
var myFunc = function() {
    getwords(zahlm2++);
}

if (myEl.addEventListener) myEl.addEventListener("click", myFunc, false);
else if (myEl.attachEvent) myEl.attachEvent("onclick", myFunc);
else myEl.onclick = myFunc;

Obviously that's pretty verbose, but it would be easy to write a quick helper function takes myEl and myFunc as inputs and handles everything for you.

You can read more about event registration at http://www.quirksmode.org/js/events_advanced.html

Upvotes: 2

Alnitak
Alnitak

Reputation: 339786

In the lines like this:

document.getElementById('makroclickm2').onclick = getwords(zahlm2++);

You're assigning the onclick handler to the result of getwords(zahlm2++), not to that function itself.

If, as I suspect, the code above actually is the getwords function, that means it's calling itself (recursively).

Instead, write:

document.getElementById('makroclickm2').onclick = function() {
   getwords(zahlm2++);
}

Upvotes: 8

Related Questions