A frontend developer interview question inspired by the book DOM Scripting

Back in high school, I read a brilliant book, DOM Scripting (first edition) by Jeremy Keith. The Chinese translation of the book had a even better title: "JavaScript DOM 编程艺术", which literally means "the art of JavaScript DOM programming". The book was the first book I bought about JavaScript. It was so enlightening that it showed me how to write JavaScript in a "good" way (graceful degradation, in particular), and it eventually turned me into a frontend developer a decade later. If you ask to me to recommend 3 books for learning JavaScript, I'll definitely have DOM Scripting in the list.

Last year, I saw a JavaScript function appeared in a question on SegmentFault, where the post author asked for some explanation on the code he posted. I wrote an answer:

The code you posted is a piece of shit. You can learn nothing from it. Actually, I've never seen any JavaScript code worse than that.

The post author replied: "I read the code from Keith's DOM Scripting". I was astonished. Recalling that DOM Scripting is such a masterpiece, I cannot believe it. After realizing the code was really extracted from the book, I felt embarrassed and woke up to the fact that JavaScript had really changed a lot over the years.

Therefore, I made it a interview question for frontend developer candidates: read the very code section, and tell me how will you improve it. Only include basic code issues (including code style issues). Don't change the logic. Don't use fancy ES5/6 things. Don't say "requestAnimationFrame API" can help the performance. Just focus on the code itself. Here's the original code, again:

function moveElement(elementID,final_x,final_y,interval) {
    if (!document.getElementById) return false;
    if (!document.getElementById(elementID)) return false;
    var elem = document.getElementById(elementID);
    if (elem.movement) {
        clearTimeout(elem.movement);
    }
    if (!elem.style.left) {
        elem.style.left = "0px";
    }
    if (!elem.style.top) {
        elem.style.top = "0px";
    }
    var xpos = parseInt(elem.style.left);
    var ypos = parseInt(elem.style.top);
    var dist = 0;
    if (xpos == final_x && ypos == final_y) {
        return true;
    }
    if (xpos < final_x) {
        var dist = Math.ceil((final_x - xpos)/10);
        xpos = xpos + dist;
    }
    if (xpos > final_x) {
        var dist = Math.ceil((xpos - final_x)/10);
        xpos = xpos - dist;
    }
    if (ypos < final_y) {
        var dist = Math.ceil((final_y - ypos)/10);
        ypos = ypos + dist;
    }
    if (ypos > final_y) {
        var dist = Math.ceil((ypos - final_y)/10);
        ypos = ypos - dist;
    }
    elem.style.left = xpos + "px";
    elem.style.top = ypos + "px";
    var repeat = "moveElement('"+elementID+"',"+final_x+","+final_y+","+interval+")";
    elem.movement = setTimeout(repeat,interval);
}

Here are some possible answers I'll give, listed in the order they appeared:

• Use var moveElement = function (...) syntax instead of function moveElement(...) to make the program more consistent (okay, it's just my personal preference).
• Leave spaces after commas in argument list (i.e. moveElement(elementID, final_x, final_y, interval) instead of moveElement(elementID,final_x,final_y,interval)). Similarly, leave spaces around operators like /.
• Either use camel case (better) or underscore case. Don't mix them up (elementID and final_x).
• Always put curly brackets around the body of if, even when there is only one statement inside (return false in this example).
• Always have consistent return type (don't return false at the beginning, but returning nothing at the end).
• Don't declare variables using var inside if blocks. JavaScript's var keyword only gives function-level locality.
• Don't use var to declare the same variable more than one time (there are var dist for multiple times in the code).
• The is no point in assigning zero to dist at the beginning.
• It's a very bad practice to pass a string as the argument to setTimeout. Pass a function instead.
• Using == to compare is discouraged. Use === instead.
• Some code quality tools (JSLint or JSHint) will ask you to supply the second argument (10) to parseInt.
• Don't call to getElementById each time moveElement is called. Call getElementById just once, and use the element itself in recurring calls.
• Similarly, don't call parseInt again and again. Put numeric values in variables instead.

Here's the revised code I'll have:

var moveElement = function (elementID, finalX, finalY, interval) {
    if (!document.getElementById) { return; }
    if (!document.getElementById(elementID)) { return; }
    var elem = document.getElementById(elementID);
    if (elem.movement) {
        clearTimeout(elem.movement);
    }
    var xpos = 0;
    var ypos = 0;
    var run = function () {
        var dist;
        if (xpos === finalX && ypos === finalY) {
            return;
        }
        if (xpos < finalX) {
            dist = Math.ceil((finalX - xpos) / 10);
            xpos = xpos + dist;
        }
        if (xpos > finalX) {
            dist = Math.ceil((xpos - finalX) / 10);
            xpos = xpos - dist;
        }
        if (ypos < finalY) {
            dist = Math.ceil((finalY - ypos) / 10);
            ypos = ypos + dist;
        }
        if (ypos > finalY) {
            dist = Math.ceil((ypos - finalY) / 10);
            ypos = ypos - dist;
        }
        elem.style.left = xpos + "px";
        elem.style.top = ypos + "px";
        elem.movement = setTimeout(run, interval);
    };
    run();
};

What will the frontend developer candidates answer? Maybe they're used to shims, polyfills, transpilers. Will they know the hard years we had with IE 6?