Creating a Game Map

I'm trying to make a kind of Dungeon Keeper clone. I got the code up and running and even supporting touch events. But I don't think it's as well optimized as it could be, so I am looking for help and best practices on helping improve my map.

You can test it out and play with it here or for full screen here.

It runs perfectly on my local machine and iPhone, but using it from JSFiddle seems to cause some problems. I'm not sure if it's a problem with JSFiddle or if my localhost plays more fast and loose with JavaScript.

```
var CanvasDisplay = function (parent, level) {
this.canvas = document.createElement("canvas");
this.canvas.width = window.innerWidth;
this.canvas.height = window.innerHeight;
parent.appendChild(this.canvas);
this.cx = this.canvas.getContext("2d");

this.cx.canvas.addEventListener("mousedown", this.mapMove);
this.cx.canvas.addEventListener("click", this.mapSelect);
//wheel event for Chrome, Safari, and Opera
this.cx.canvas.addEventListener("mousewheel", this.mapZoom);
//wheel event for Firefox
this.cx.canvas.addEventListener("DOMMouseScroll", this.mapZoom);

//touch events
this.cx.canvas.addEventListener("touchstart", this.mapTouchStart);
this.cx.canvas.addEventListener("touchmove", this.mapMoveTouch);

this.touch = {
x: null,
y: null,
hypotenuse: null
};

this.level = {
height: level.length,
width: level[0].length,
scale: 100
};

this.view = {
x: this.canvas.width / 2 - (this.level.width * this.level.scale / 2),
y: this.canvas.height / 2 - (this.level.height * this.level.scale / 2),
width: this.canvas.width,
height: this.canvas.height,

move: false,
select: {}
};
};

CanvasDisplay.prototype.mapSelect = function (event) {
if (display.view.move === true) {
display.view.move = false;
return;
}

var pos = displ

You have a lot of public fields.

By that, I mean that you set a lot of properties of the CanvasDisplay class to public (you use the this keyword to create them).

It is better practice make as many properties as you can private. That means that you use the var keyword when you create them, and that you just reference to them as their name without the this keyword.

In a couple of the methods of CanvasDisplay, you catch an event and call preventDefault to stop it from performing it's default behavior.

I'm unsure if this will make a difference, but it might be better to move that function call to the very top of the function; that way (I think) the default behavior of the event is more likely to not be executed

In the function end of the function trackDrag of the function CanvasDisplay.prototype.mapMove, you have a parameter to catch the event that is fired when the function is fired.

Although, you don't use that event variable any where in that function.

In the function CanvasDisplay.prototype.mapSelect, in the first conditional of the function, if it passes, you just return.

You don't return any value.

In that situation, it would be better practice to return null, rather than to not return anything at all.

StackExchange Code Review Q#83821, answer score: 3