3
\$\begingroup\$

Can someone let me know if this is a proper (and efficient) way of doing it? It works, but I wanted to understand if there's a way it be made better (I can use a for loop for the clones, but my browser gets stuck if I do it).

Here's the fiddle

window.onload = function() {
  rot();
}
var rot = function(){
var paper = new Raphael(document.getElementById('canvas_container'), 500, 500);
var c1 = paper.circle(250,250,150).attr({"stroke-width":2});
var c2 = paper.circle(250, 250, 5).attr({"fill":"black"});
var lMark = paper.path("M 250 250 m 0 -130 l 0 -20").attr({"stroke-wid th":2}); 
lMark.clone().attr({transform:"r90 250 250"});
lMark.clone().attr({transform:"r180 250 250"});
lMark.clone().attr({transform:"r270 250 250"});
lMark.clone().attr({transform:"r360 250 250"});
var d = new Date();
var hourAngle = (d.getHours() * 60 + d.getMinutes())/2, minAngle = (d.getMinutes() * 6), secAngle = (d.getSeconds() * 6);
var secHand = paper.path("M 250 250, l 0 -110, M 250 250, l 0 15").attr({"stroke":"red", "stroke-width":2});
var minHand = paper.path("M 250 250, l 0 -120").attr({'stroke-width':2}).transform("r" + minAngle + " 250 250");
var hourHand = paper.path("M 250 250, l 0 -80").attr({'stroke-width':3, 'stroke-linecap':'round'}).transform("r" + hourAngle + " 250 250");
var secAnim = Raphael.animation({transform: "r360 250 250"}, 60000).repeat(Infinity);
var minAnim = Raphael.animation({transform: "r360 250 250"}, 3600000).repeat(Infinity);
var hourAnim = Raphael.animation({transform: "r360 250 250"}, 43200000).repeat(Infinity);
secHand.animate(secAnim); 
minHand.animate(minAnim);
hourHand.animate(hourAnim);
}
\$\endgroup\$
3
\$\begingroup\$

I think not, this is how it is meant to be.

That being said, you

  • Have a few missing semicolons
  • Capture c1 & c2 for no good reason
  • Calculate secAngle for no good reason
  • Have a veritable legion of hard coded mystical constants
  • Have a few weirdly named variables

After some thought, it appears that the biggest problem with this code is that you count on the animation to take 0 (milli)seconds and the execution/scheduling of the animations is instantaneous. Have you tried keeping this clock up for an hour or more? I would expect there to be discrepancies between the real time and the animated clock.

\$\endgroup\$
  • \$\begingroup\$ Thank you. Can you help me know what I can do to make it better? While I think I make stuff that works, they seem to be built in the wrong way. Even a link to a reference would help. \$\endgroup\$ – sarath Jun 28 '14 at 4:16
1
\$\begingroup\$

For readability, it would be nice if you changed the name of the variable d to date instead.

To fix the syncing issue, you could set an interval using setInterval(), which is a built-in JavaScript function.

\$\endgroup\$

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.