Overlap/hit test in javascript

canvas, html, html5-canvas, javascript

Solution

Some things before beginning:

- Don't create arrays with `new Array()`, unless you specify the initial length. Use `[]`;

- Don't iterate through arrays using `for...in`: use a standard `for` with a counter. This is more a good practice;

- Converting numbers into strings and converting back to numbers is useless and expensive. Use a small array to store both values;

- Don't use "magic numbers", i.e. number with a precise value but hard to recognize immediately. Use named "constants" or put a comment near each of them telling what they mean, for future maintenance.

Ok, let's see the code.

if(((x > (pp[0] - 24)) && (x < (pp[0] + 24))) && ((y > (pp[1] - 24)) && (y < (pp[1] + 24))))

Honestly, what is this? I'd call it a cranky and obscure snippet. Recall what you've learnt at school:

var dx = pp[0] - x, dy = pp[1] - y;
if (dx * dx + dy * dy < 400) return true;

Isn't it much clearer?

Let's see the whole function:

function canvasDraw () {
    var c = document.getElementById("canvas");
    var w = window.innerWidth;
    var h = window.innerHeight;
    c.width = w;
    c.height = h;

    var ctx = c.getContext("2d");
    ctx.clearRect(0,0,c.width, c.height);
    // Lolwut?
    // var abc = 0;

    var colours = ["rgb(0,100,0)", "rgb(51,102,255)"];
    var positions = [];


    function hitTest(x, y) {
        for (var j = 0; j < positions.length; j++) {
            var pp = positions[j];
            var dx = pp[0] - x, dy = pp[1] - y;
            if (dx * dx + dy * dy < 400) return true;
        }
        return false;
    }


    // You declare the radius once and for all
    var radius = 10;
    // Declare the local scoped variables. You forgot i
    var x, y, i;
    for (i=0; i<200; i++) {

        // How about a do...while instead of a while?
        do {
            var x = Math.floor(Math.random()*c.width);
            var y = Math.floor(Math.random()*c.height);
        // Testing with === is faster, always do it if you know the type
        // I'll let it here, but if the type is boolean, you can avoid testing
        // at all, as in while (hitTest(x, y));
        } while (hitTest(x, y) === true);

        positions.push([x, y]);

        // This instruction is useless
        // var r = radius.toString();

        var b = colours[Math.floor(Math.random()*colours.length)];

        circle(ctx,x,y, radius, b);

    }   
}

BEWARE, though, that depending on your canvas size, there could be no more room for another circle, so it could end in an infinite loop. Try to put 200 circles with a radius of 10 inside a 40x40 box... There should be another test to do, and that could be complicated.

Problem

Original source