1
\$\begingroup\$

I've written this bit of JavaScript to just learn basics of drawing on/interacting with the HTML5 canvas element.

Just want to make sure I'm doing things "correctly" in case I have any glaring code gaffs or there's a more efficient way.

function start() {
  canvas = document.getElementById("area");

  // initiate ball
  ball.init({
    context: canvas.getContext('2d'),
    color: "#F33",
    radius: 30,
  });
}

var ball = (function() {
  var ball;

  var mouseMoveEvent;
  var prevMouseMoveEvent;

  // set default options
  var default_options = {
    context: "", // required
    radius: 20,
    color: "#F33",
    startX: window.innerWidth / 2,
    startY: window.innerHeight / 2
  };

  return {
    draw: function() {
      // prep canvas
      ball.o.context.canvas.width = window.innerWidth;
      ball.o.context.canvas.height = window.innerHeight;

      //capture current position values
      var cur_x = ball.posX;
      var cur_y = ball.posY;

      //capture current context dimensions
      var ctx_width = ball.o.context.canvas.width;
      var ctx_height = ball.o.context.canvas.height;


      if (ball.isGrabbed) {
        //-------------- track ball with mouse when grabbed
        mouseOffsetX = mouseMoveEvent.x - prevMouseMoveEvent.x;
        mouseOffsetY = mouseMoveEvent.y - prevMouseMoveEvent.y;

        ball.posX += mouseOffsetX;
        ball.posY += mouseOffsetY;

        // save previous mouse move state
        prevMouseMoveEvent = mouseMoveEvent;
      } else {
        //-------------- bounding
        var x_reach = Math.abs(ball.iterX) + ball.o.radius;
        var y_reach = Math.abs(ball.iterY) + ball.o.radius;

        if ((cur_x + x_reach) > ctx_width || (cur_x - x_reach) < 0)
          ball.iterX = -(.70 * ball.iterX);

        if ((cur_y + y_reach) > ctx_height || (cur_y - y_reach) < 0)
          ball.iterY = -(.70 * ball.iterY);

        ball.iterX *= .999;
        ball.iterY *= .999;
        ball.posX += ball.iterX;
        ball.posY += ball.iterY;
      }

      //-------------- protect browser borders
      // North
      if (ball.posY - ball.o.radius < 0)
        ball.posY = ball.o.radius;
      // South
      else if (ball.posY + ball.o.radius > ctx_height)
        ball.posY = ctx_height - ball.o.radius;
      // East
      else if (ball.posX + ball.o.radius > ctx_width)
        ball.posX = ctx_width - ball.o.radius;
      // West
      else if (ball.posX - ball.o.radius < 0)
        ball.posX = ball.o.radius;

      //-------------- draw
      ball.o.context.beginPath();
      ball.o.context.fillStyle = ball.o.color;
      ball.o.context.arc(ball.posX, ball.posY, ball.o.radius, 0, Math.PI * 2, true);
      ball.o.context.closePath();
      ball.o.context.fill();
    },
    mouseDown: function(e) {
      // grab ball
      if (ball.o.context.isPointInPath(e.x, e.y)) {
        prevMouseMoveEvent = e;
        ball.isGrabbed = true;
      }
    },
    mouseUp: function(e) {
      // release
      if (ball.isGrabbed) {
        // set iter speed based on mouse speed on release
        ball.iterX = mouseMoveEvent.x - prevMouseMoveEvent.x;
        ball.iterY = mouseMoveEvent.y - prevMouseMoveEvent.y;

        ball.isGrabbed = false;
      }
    },
    mouseMove: function(e) {
      if (ball.o.context.isPointInPath(e.x, e.y)) {
        document.body.style.cursor = "move";
      } else {
        document.body.style.cursor = "default";
      }
      mouseMoveEvent = e;
    },
    init: function(options) {
      ball = this;

      //load up defaults
      ball.o = default_options;

      // merge in user options that exist
      for (var attr in options) {
        ball.o[attr] = options[attr];
      };

      // set starting values
      ball.posX = ball.o.startX;
      ball.posY = ball.o.startY;
      ball.iterX = 0;
      ball.iterY = 0;
      ball.isGrabbed = false;

      // attach events
      window.onmousedown = ball.mouseDown;
      window.onmouseup = ball.mouseUp;
      window.onmousemove = ball.mouseMove;

      // start
      setInterval(ball.draw, 1);
    },
  };

})();
body {
  margin: 0px;
}

#area {
  width: 100%;
  height: 100%;
  margin: 0px;
  background: #FF7
}
<body onLoad="start();">
  <canvas id="area"></canvas>
</body>

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

This looks pretty good. In your start function, the canvas variable should be declared with a var; implicit globals are one of the Worst Parts.

function start()
{
    var canvas = document.getElementById("area");

    // ...
}

Second, the var statement can declare more than one variable at a time, so lines like

var cur_x = ball.posX;
var cur_y = ball.posY;

should be

var cur_x = ball.posX, cur_y = ball.posY;

Third, this line

for (var attr in options) { ball.o[attr] = options[attr]; };
  1. has an extra semicolon at the end and more importantly
  2. doesn't do a hasOwnProperty check.

The vast majority of the time, when you are iterating over object properties, you should check to make sure the properties actually exist on the object instead of the prototype chain. More more information read this article by Douglas Crockford. So it should look like:

for (var attr in options) {
    if (options.hasOwnProperty(attr))
        ball.o[attr] = options[attr];
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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