IE gets errors on controls.html

Same as:

http://dev.rubyonrails.org/ticket/7349

which is open for 6 months without a test case -- but we know this is breaking in code other than Openlayers, and we also know that testing it is hard because IE is crap.

14:33:33 &lt; crschmidt> tschaub: Anyway, I'm in favor of a try/catch -- seems like this is just IE stupidity, which we shouldn't bother to fight with, since it seems to be at least an issue that's not directly related to OpenLayers<br /> 14:45:01 &lt; tschaub> crschmidt: I agree

Going to work on a patch based on that.

Attached patch tested to fix problem in IE7. Sucks to have to fall back to just hacking around things, but at least we're not alone.

smaller try/catch

Ok, I'll submit we need to accommodate IE here with a try/catch. As long as we're touching this code, can we make it more readable?

Perhaps it's just me, but this do/while with assignment as condition in the while stuff creeps me out a bit.

The second patch puts the try/catch around the single line that offends IE and restructures the loops a bit.

All tests pass in IE and FF - and as a bonus, controls.html works in IE.

Can we stick a comment on:

for(element=forElement; element; element=element.parentNode) {

or change it somehow for better readability? Hurts my head just looking at it.

at the risk of throwing another chef into the kitchen, this could be another version of the patch... maybe satisfying ?

Note that the purpose of this ticket is to get examples/controls.html working in IE. The iecrash.3.patch does not do this.

if you don't like for loops

Tim --

Yeah, Erik and I chatted about it, he just got a bit mixed up.

I like while loops better than for loops in this case :) Looks good -- go ahead and commit when you're ready.

Happy to commit it this way.

For the record, here's my thinking on loop constructs:

• for loops - good
• while loops - okay
• do while loops - bad

My reasoning has to do with maintenance and readability.

Here's the structure of a for loop

for(initialize; test; increment) {
    statements;
}

Here's the structure of a while loop

initialize;
while(test) {
    statements;
    increment;
}

Here's the structure of a do/while loop

initialize;
do {
    // always executed once
    statements;
    increment;
} while (test);

My least favorite structure for do/while (used in OL) is

initalize;
do {
    statements;
} while (increment as test);

These may not look that different in the above examples. My reasoning has to do with real life code. The block containing statements is typically large. In Layer.Grid.initGriddedTiles the statements are 34 lines long. This means that the test determining how long a loop iterates is many lines *below* the top of the loop. Given a culture that reads top to bottom, I think this goes against intuition.

Also, the Util do/while loops used to use assignment as the test condition in the while statement. This is particularly bad for debugging.

A while loop suffers from the same separation problem - the increment may be many lines below the test given many statement lines - loops that run indefinitely are hard to debug when code is separated by many lines.

A for loop puts the initialize, test, and increment all on one line - readable (if you know the for statement syntax) and maintainable.

Resolved with r4061.

Replying to tschaub:

Happy to commit it this way. For the record, here's my thinking on loop constructs: * for loops - good * while loops - okay * do while loops - bad My reasoning has to do with maintenance and readability. Here's the structure of a for loop {{{ for(initialize; test; increment) { statements; } }}} Here's the structure of a while loop {{{ initialize; while(test) { statements; increment; } }}} Here's the structure of a do/while loop {{{ initialize; do { // always executed once statements; increment; } while (test); }}} My least favorite structure for do/while (used in OL) is {{{ initalize; do { statements; } while (increment as test); }}} These may not look that different in the above examples. My reasoning has to do with real life code. The block containing statements is typically large. In Layer.Grid.initGriddedTiles the statements are 34 lines long. This means that the test determining how long a loop iterates is many lines *below* the top of the loop. Given a culture that reads top to bottom, I think this goes against intuition. Also, the Util do/while loops used to use assignment as the test condition in the while statement. This is particularly bad for debugging. A while loop suffers from the same separation problem - the increment may be many lines below the test given many statement lines - loops that run indefinitely are hard to debug when code is separated by many lines. A for loop puts the initialize, test, and increment all on one line - readable (if you know the for statement syntax) and maintainable.

These are sage thoughts señor schaub. My only qualm is that the unconventional use of the for loop tends to confuse people who are used to seeing for loops only used in the standard

    for(var i=0; i < length; i++)  {
        //blah
    }

format. Although it's perfectly valid to do something like:

    for(var elem = foo; elem; elem = elem.parentNode) {
        //blah
    }

I think most people are more comfortable with a while loop in that case. The condition part of the statement is more readily readable due to the english ("while") context.

    var elem = foo;
    while(elem) {
        //blah
        elem = elem.parentNode;
    }

I do think you make a valid point regarding the length of the //blah section and how that can cause readability issues when it gets long... but I think 34 line while loops should be an *extreme* exception. The gridding code is I think a valid exception as it's some harcore math bits, but in general I think sections in braces should be no longer than 10 or so lines -- half a screen max or something like that.