3
\$\begingroup\$

I have a small animation with Raphael using a large amount of code. Is there any way to reduce it?

Demo

window.onload = function () {
    var elements = document.querySelectorAll('.colbr');
    for (i=0; i<elements.length; i++) {
        var loadSpinner = spinner(elements[i], 7, 12, 10, 3, "#000");
    }
};

function spinner(holderid, R1, R2, count, stroke_width, colour) {
    var sectorsCount = count || 12,
        // color = colour || "#000",
        color = "#000",
        width = stroke_width || 15,
        r1 = Math.min(R1, R2) || 35,
        r2 = Math.max(R1, R2) || 60,
        cx = r2 + width,
        cy = r2 + width,
        r = Raphael(holderid, r2 * 2 + width * 2, r2 * 2 + width * 2),

        sectors = [],
        opacity = [],
        beta = 2 * Math.PI / sectorsCount,

        pathParams = {stroke: color, "stroke-width": width, "stroke-linecap": "round"};
        Raphael.getColor.reset();
    for (var i = 0; i < sectorsCount; i++) {
        var alpha = beta * i - Math.PI / 2,
            cos = Math.cos(alpha),
            sin = Math.sin(alpha);
        opacity[i] = 1 / sectorsCount * i;
        sectors[i] = r.path([["M", cx + r1 * cos, cy + r1 * sin], ["L", cx + r2 * cos, cy + r2 * sin]]).attr(pathParams);
        if (color == "rainbow") {
            sectors[i].attr("stroke", Raphael.getColor());
        }
    }
    var tick;
    (function ticker() {
        opacity.unshift(opacity.pop());
        for (var i = 0; i < sectorsCount; i++) {
            sectors[i].attr("opacity", opacity[i]);
        }
        r.safari();
        tick = setTimeout(ticker, 1000 / sectorsCount);
    })();
    return function () {
        clearTimeout(tick);
        r.remove();
    };
}
\$\endgroup\$
2
\$\begingroup\$

I really like what you wrote there, though I am afraid at this point you ought to expand the code, not reduce it.

  • r, r1, r2, cx, cy, beta are all variables that need better ( more meaningful ) names
  • You need to indent your code, just hit TidyUp in your fiddle and you will see
  • This is not part of your core code, but you should not simply assign something to window.onload, you should use addEventListener
  • Your code had support for using custom colors, and then you took it out, if you really want to take that out, then you should also take out the parameter, the commented out code and the if (color == "rainbow"){ statement
  • in your var statement, you could simply do cy = cx
  • be consistent in your name, and use lowerCamelCase: stroke_width -> strokeWidth
  • You did not declare i with var
  • While straightforward, your code has no comments at all..
\$\endgroup\$
  • 1
    \$\begingroup\$ You could use <kbd> instead of the tag notation, for tidyup. \$\endgroup\$ – Marc-Andre Mar 13 '14 at 18:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.