9
\$\begingroup\$

I am trying to create a builder for table tennis exercises. User Blindman67 helped me here. Now I'm trying to clean up the code before continuing to work on it. Please tell me how to clean up the code so JSLint does not give any more warnings. If you notice any other things, feel free to tell me. I'm a newbie :)

My main problems are:

  • Using "use strict"; and this in the same function gives a "strict violation". I would like to use "use strict";, but does that mean every this has to be rewritten to something else?
  • JSLint doesn't like the bitwise operators. What is the "JSLint-approved"-way to say |= and &= (lines 292 and 294)?
  • JSLint also doesn't recognize the arrow operator => (lines 530 and 533). I managed to replace one in line 318 with an extra function, but I cant figure out how to do it in the lines 530 and 533.
  • The createAddTable function depends on drawEmpty, drawEmpty depends on addTable and addTable depends on createAddTable. Now JSLint tells me to put one in front of the other so I dont call something I dont have defined yet. How can I solve this?

// contains an array of tables.
var tableArray = [];
// App constants all up top
var GLOBAL_SCALE = 1;
var SHOW_HELP = true; // set to false to have the help turned off
var SHADOW = 'rgba(0,0,0,0.8)';
var WHITE = "white";
var TABLE_REFRESH_DELAY = 50; // Time in millisecond befor updating DOM for table add and remove
var FONT = {
  face: "px Arial",
  size: Math.max(10, 18 * GLOBAL_SCALE),
  fill: WHITE
};
var TABLE = {
  width: 223 * GLOBAL_SCALE, // size of table
  height: 314 * GLOBAL_SCALE,
  tables: document.getElementById("tables"),
  image: { // table image styles
    shadow: SHADOW,
    shadowBlur: 20 * GLOBAL_SCALE,
    fill: "#2e3f73",
    lines: WHITE,
    font: FONT,
    cursor: "default"
  },
  empty: { // empty table styles
    inset: 30 * GLOBAL_SCALE, // amount box is inset
    lines: 'rgba(255,255,255,0.5)',
    lineWidth: 8 * GLOBAL_SCALE,
    shadow: SHADOW,
    shadowBlur: 20 * GLOBAL_SCALE,
    font: FONT,
    cursor: "pointer",
    highlightAmount: 0.3 // amount to highlight empty table when mouse over 0 none 1 full
  },
  arrow: { // arrow styles
    width: 15 * GLOBAL_SCALE, // arrow width
    shadow: SHADOW,
    shadowBlur: 10 * GLOBAL_SCALE,
    // custom cursor
    cursor: "url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADcAAAAVCAYAAADiv3Z7AAACtUlEQVRYha2Ye1NTMRDFf/KqLY/SlvIolAJFqSKOoqI4o+P3/1LXP3oi23XvTUrpzJm52Zxs9mRzczeFl/u9KkDp2FV5xb/IURT42hIoGbcsp1hMGrxegw1hU9gCWsDrDFriWrTc2FYDL/K1qZiS2KywdRO0DcAiTdIBtoEdYFfYC7Arzo7426a96+B53n/idBTDlha7VmAStiGyD9wjiegCPWAAHABDg0NhqL6B0BcGZpyF5fQM98D52lcc7SaBXlhbArrGeRTQEDgCKoNzwdsq4Aw4BUZqn8qWeGOhMryRGWt9jrRwfSMwbdF/72F6SFuxLXLPORppwspMPgYq+0v93mb63rh2OFbPU9sO5hlrcXtKRstnz2atxXwb9oCqEhYmWrRdR4F6mxdQIzqyfWjwOQOutMgHzF+RlL0FcTZrXaV77MU4YZ+EXIC/InuTuIL+B809Ay6UvX3mZ4TdmgviOiIdARPgxooywr4BX4TGbRkFvoo40/4MvHPi/HtXK+5CKxOJ+y6BXwsC/ePav18gcz+AeyeuS2ZbenFN2zJlLxuUa4fbtIkTtFfOnBfzENjum8TZbWQ4j5nt9jPgPEY+KXzn1ng6UPaYnz7p+1JphW7R6WVsM6FS/x1Ph41d5dT+KB+3at/JVrn+9/Jf6fnWjEm4ofC0jD4Fhxo4kZOpwxVwaTBl/g17q4lnDjfqnxp/17IlXMt+qYxc6NnyLafoO1f3ER8Cxyx+xG3lcKL+E9N/pknPtTATPY/VNwr8nbFYvRw7nDj+qWzZCsVnr6T8snVfj//rv1SaRbVlPxhj0Wf+/lhE3MTL1paRwFzh7Kv2qMqPbgUR3/vsOET+l7oVWIElV56Su9ky9znvczPjf+n7nBUZ3YKjW/FzbuO5MXV/U6x8E68TmrP5vuf+h1IaT5b7F+ZaSjBzrT+rAAAAAElFTkSuQmCC') 10 11, pointer",
    fill: "#ffb900",
    highlight: "#ffdc44",
    lineWidth: 1,
    line: "#ffdc44",
    lineHigh: "#ffed55",
    head: 30 * GLOBAL_SCALE, // arrow head width
    minSize: 5 // min size arrow can be if smaller then arrow is not created
  },
  DOM: { // variouse dom setting for table canvas and div tags
    display: "inline-block",
    canvasClass: "table",
    zIndex: 1
  },
  closeIcon: { // styles for reandering and display close icon
    size: 32 * GLOBAL_SCALE,
    fill: "red",
    lines: WHITE,
    lineWidth: Math.max(1, 2 * GLOBAL_SCALE),
    shadow: SHADOW,
    shadowBlur: 20 * GLOBAL_SCALE,
    cursor: "pointer",
    pos: {
      x: 1, // as fractions
      y: 0
    }
  },
  help: { // text help
    empty: ["Click here to", "add a new table"],
    active: ["Click to drag arrows"],
    activeArrow: ["Right click on arrow", "to remove it"],
    closeTable: ["close table", "move to top right", "click Close Icon"]
  }
};
var MOUSE = { // event contains a list of mouse event to listen to 
  buttonMasks: [1, 2, 4, 6, 5, 3],
  events: ["mousemove", "mousedown", "mouseup", "mouseout", "mouseover", "contextmenu"]
}; // contextmenu is included as that needs to be blocked for right button events
var helpItemsUsed = {
  empty: false,
  active: false,
  activeArrow: false,
  closeTable: false
};
var turnOffHelp = function() {
  "use strict";
  helpItemsUsed.empty = true;
  helpItemsUsed.active = true;
  helpItemsUsed.activeArrow = true;
  helpItemsUsed.closeTable = true;
};
if (!SHOW_HELP) {
  turnOffHelp();
}
// returns distance of point p to line segment x, y,xx,yy
var distFromLine = function(px, py, x, y, xx, yy) {
  "use strict";
  var vx, vy, pvx, pvy, lx, ly, u;
  vx = xx - x;
  vy = yy - y;
  pvx = px - x;
  pvy = py - y;
  u = (pvx * vx + pvy * vy) / (vy * vy + vx * vx);
  if (u >= 0 && u <= 1) {
    lx = vx * u;
    ly = vy * u;
    return Math.sqrt(Math.pow(ly - pvy, 2) + Math.pow(lx - pvx, 2));
  }
  // closest point past ends of line so get dist to closest end
  return Math.min(Math.sqrt(Math.pow(xx - px, 2) + Math.pow(yy - py, 2)), Math.sqrt(Math.pow(x - px, 2) + Math.pow(y - py, 2)));
};
// set up functions create images and do other general setup
function setupContext(ctx, descript) { // sets common context settings
  "use strict";
  ctx.shadowBlur = descript.shadowBlur;
  ctx.shadowColor = descript.shadow;
  ctx.strokeStyle = descript.lines;
  ctx.fillStyle = descript.fill;
  ctx.lineWidth = descript.lineWidth;
  ctx.lineCap = "round";
  if (descript.font) {
    ctx.font = descript.font.size + descript.font.face;
  }
}

function createTableImage() { // create image of table but why write a comment when the function tells it all???
  "use strict";
  var table = document.createElement("canvas"),
    ctx = table.getContext("2d"),
    scaleX = TABLE.width / 223, // get the scale compared to original layout
    scaleY = TABLE.height / 314; // get the scale compared to original layout

  table.height = TABLE.height;
  setupContext(ctx, TABLE.image);

  ctx.fillStyle = TABLE.image.fill;
  ctx.fillRect(35.25 * scaleX, 20 * scaleY, 152.5 * scaleX, 274 * scaleY);
  ctx.fillStyle = TABLE.image.lines; // lines
  ctx.fillRect(111.35 * scaleX, 20 * scaleY, 0.3, 274 * scaleY); // middle line
  ctx.fillRect(35.25 * scaleX, 20 * scaleY, 2, 274 * scaleY); // left side
  ctx.fillRect(185.75 * scaleX, 20 * scaleY, 2, 274 * scaleY); // right side
  ctx.fillRect(35.25 * scaleX, 20 * scaleY, 152.5 * scaleX, 2); // top base line
  ctx.fillRect(35.25 * scaleX, 292 * scaleY, 152.5 * scaleX, 2); // bottom base line
  ctx.fillRect(20 * scaleX, 156 * scaleY, 183 * scaleX, 2); // net
  return table;
}

function createEmptyImage() { // empty table image
  "use strict";
  var i = TABLE.empty.inset,
    image = document.createElement("canvas"),
    w = image.width = TABLE.width,
    h = image.height = TABLE.height,
    ctx = image.getContext("2d");
  setupContext(ctx, TABLE.empty);
  ctx.strokeRect(i, i, w - i * 2, h - i * 2);
  ctx.beginPath();
  ctx.moveTo(i * 2, i * 2);
  ctx.lineTo(w - i * 2, h - i * 2);
  ctx.moveTo(i * 2, h - i * 2);
  ctx.lineTo(w - i * 2, i * 2);
  ctx.stroke();
  return image;
}

function createCloseImage() { // create close icon
    "use strict";
    var S = TABLE.closeIcon.size,
      s = S * 0.5,
      c = s * 0.4, // cross dist from center
      sb = TABLE.closeIcon.shadowBlur,
      l = TABLE.closeIcon.lineWidth,
      image = document.createElement("canvas"),
      cx = s + sb / 2, // add half blur to get center
      cy = s + sb / 2,
      ctx = image.getContext("2d");
    // Image must include shadowblur
    image.width = S + sb; // add blur to size
    image.height = S + sb;
    setupContext(ctx, TABLE.closeIcon);
    ctx.beginPath();
    ctx.arc(cx, cy, s - l, 0, Math.PI * 2);
    ctx.fill();
    ctx.stroke();
    ctx.beginPath();
    ctx.moveTo(cx - c, cy - c);
    ctx.lineTo(cx + c, cy + c);
    ctx.moveTo(cx - c, cy + c);
    ctx.lineTo(cx + c, cy - c);
    ctx.stroke();
    return image;
  }
  // create the images
var tableImage = createTableImage();
var closeIcon = createCloseImage();
var emptyTableImage = createEmptyImage();
// draws a arrow a is the arrow object
function drawArrow(ctx, a) {
    "use strict";
    var s = TABLE.arrow, // get arrow style
      x = a.x,
      y = a.y,
      vx = a.xx - x,
      vy = a.yy - y,
      dir = Math.atan2(vy, vx),
      len = Math.sqrt(vx * vx + vy * vy),
      w = s.width / 2,
      h = Math.min(len, s.head); // ensure arrow head no bigger than arrow length	
    // ctx.save();
    ctx.setTransform(1, 0, 0, 1, x, y);
    ctx.rotate(dir);
    h /= 2;
    if (a.highlight) {
      ctx.fillStyle = s.highlight;
      ctx.strokeStyle = s.lineHigh;
    } else {
      ctx.fillStyle = s.fill;
      ctx.strokeStyle = s.line;
    }
    ctx.lineWidth = s.lineWidth;
    ctx.save();
    ctx.shadowBlur = s.shadowBlur;
    ctx.shadowColor = s.shadow;
    ctx.beginPath();
    ctx.moveTo(0, -w / 2);
    ctx.lineTo(len - h - h, -w);
    ctx.lineTo(len - h - h, -h);
    ctx.lineTo(len, 0);
    ctx.lineTo(len - h - h, h);
    ctx.lineTo(len - h - h, w);
    ctx.lineTo(0, w / 2);
    ctx.closePath();
    ctx.fill();
    ctx.stroke();
    ctx.restore();
  }
  // display help text for table

function drawHelpText(ctx, text, style) {
    "use strict";
    var i,
      len = text.length,
      y = ctx.canvas.height / 2 - len * style.font.size * 1.2,
      yy = y + 1;
    ctx.font = style.font.size + style.font.face;
    ctx.textAlign = "center";
    ctx.textBaseline = "middle";
    ctx.strokeStyle = "#000";
    ctx.lineWidth = 2;
    for (i = 0; i < len; i += 1) {
      ctx.strokeText(text[i], ctx.canvas.width / 2 + 1, yy);
      yy += TABLE.empty.font.size * 1.2;
    }
    ctx.fillStyle = style.font.fill;
    for (i = 0; i < len; i += 1) {
      ctx.fillText(text[i], ctx.canvas.width / 2, y);
      y += TABLE.empty.font.size * 1.2;
    }
  }
  //------------------------------------------------------------
  // functions for table

function drawClose() { // draws close icon. Fades in the close mouse is
  var ctx = this.ctx,
    w = closeIcon.width,
    grow = w * 0.1,
    x = (this.width - w) * TABLE.closeIcon.pos.x,
    y = (this.height - w) * TABLE.closeIcon.pos.y,
    ic_x = x + w / 2, // icon x and y
    ic_y = y + w / 2,
    dist = Math.sqrt(Math.pow(this.mouse.x - ic_x, 2) + Math.pow(this.mouse.y - ic_y, 2));

  if (dist < TABLE.closeIcon.size / 2) {
    this.mouseOverClose = true;
  } else {
    this.mouseOverClose = false;
  }
  ctx.globalAlpha = 1 - (Math.min(100, (dist - w * 2)) / 100);
  if (this.mouseOverClose) {
    ctx.drawImage(closeIcon, x - grow, y - grow, w + grow * 2, w + grow * 2);
  } else {
    ctx.drawImage(closeIcon, x, y);
  }
  ctx.globalAlpha = 1;
}

function mouseEvent(e) {
  var m = this, // lazy programer short cut
    t = e.type,
    bounds = m.element.getBoundingClientRect();
  m.x = e.clientX - bounds.left;
  m.y = e.clientY - bounds.top;
  if (t === "mousedown") {
    m.button |= MOUSE.buttonMasks[e.which - 1];
  } else if (t === "mouseup") {
    m.button &= MOUSE.buttonMasks[e.which + 2];
  } else if (t === "mouseout") {
    m.button = 0;
    m.over = false;
    m.table.mouseOver();
  } else if (t === "mouseover") {
    m.over = true;
    m.table.mouseOver();
  }
  e.preventDefault();
}

function removeTable(table) { // remove table from dom
  "use strict";
  table.mouse.remove(); // deactivate moue events
  TABLE.tables.removeChild(table.div); // remove from DOM
  table.dead = true; // flag as dead to be removed from table array
}

function updateTables() { // Updates tables. Removes any dead tables from table array
  "use strict";
  var closeTables = [],
    i;

  closeTables = tableArray.filter(function(t) {
    return !t.active;
  });
  while (closeTables.length > 1) {
    removeTable(closeTables.shift());
  }
  for (i = 0; i < tableArray.length; i += 1) {
    if (tableArray[i].dead) {
      tableArray.splice(i, 1);
      i -= 1;
    }
  }
}

function drawTable() { // darw the table all states
  var ctx = this.ctx,
    minDist = TABLE.arrow.width, // this sets the max distance mouse can be for it to highlight an arrow
    dist = 0,
    i;

  ctx.clearRect(0, 0, ctx.canvas.width, ctx.canvas.height);
  if (this.active) {
    ctx.drawImage(tableImage, 0, 0);
    if (this.mouse.over) {
      if (!this.dragging) { // Dont draw close icon while draggin
        this.drawCloseIcon();
      }
      if (this.mouseOverClose && !this.dragging) { // if not dragging and mouse over close
        this.cursor = TABLE.closeIcon.cursor; // set cursor
        if (this.mouse.button === 1) { // bit field  is mouse left down
          this.buttonDown = true;
        } else if (this.buttonDown) { // only close if mouse moves up while over close.
          this.active = false;
          helpItemsUsed.closeTable = true;
          this.buttonDown = false;
          setTimeout(updateTables, TABLE_REFRESH_DELAY);
        }
      } else { // not over close
        // if near a arrow and mouse button right is down delete the arrow
        if (this.closestArrowIndex > -1 && this.mouse.button === 4) { // but field Only button right down
          this.arrows.splice(this.closestArrowIndex, 1);
          this.closestArrowIndex = -1;
          this.mouse.button = 0; // turn mouse click off
          helpItemsUsed.activeArrow = true; // flag arrow delete help as used
        } else if (this.mouse.button === 1) { // bit field  if down start dragging new arroe
          if (!this.dragging) { // Start of drag create arrow
            this.arrows.push({
              x: this.mouse.x,
              y: this.mouse.y,
              xx: this.mouse.x,
              yy: this.mouse.y
            });
            this.currentArrow = this.arrows[this.arrows.length - 1];
            this.dragging = true;
          } else { // during drag move arrow endpoint
            helpItemsUsed.active = true; // flag arrow help as used
            this.currentArrow.xx = this.mouse.x;
            this.currentArrow.yy = this.mouse.y;
          }
        } else { // mouse up
          if (this.dragging) { // is dragging then must be a arrow
            // if arrow added is smaller than 2 pixels then remove it;
            if (Math.abs(this.currentArrow.xx - this.currentArrow.x) < TABLE.arrow.minSize && Math.abs(this.currentArrow.y - this.currentArrow.yy) < TABLE.arrow.minSize) {
              this.arrows.length -= 1;
            }
            this.currentArrow = null;
            this.dragging = false;
          }
        }
        this.cursor = TABLE.image.cursor; // set cursor tp table standard
      }
    }
    if (this.closestArrowIndex > -1 && !this.dragging) { // is mouse near arrow 
      this.cursor = TABLE.arrow.cursor; // yes set cursor for arrow
    }
    this.closestArrowIndex = -1;
    for (i = 0; i < this.arrows.length; i += 1) { // test all arrow
      var a = this.arrows[i];
      drawArrow(ctx, a); // draw the arrow
      a.highlight = false; // turn off highlight
      dist = distFromLine(this.mouse.x, this.mouse.y, a.x, a.y, a.xx, a.yy); // get distance from mouse
      if (dist < minDist) { // is closer than any other arrow
        this.closestArrowIndex = i; // yes remember the index
        minDist = dist;
      }
    }
    if (this.closestArrowIndex > -1 && this.mouse.over) { // is a arror close to mouse
      this.arrows[this.closestArrowIndex].highlight = true; // highlight it
    }
    ctx.setTransform(1, 0, 0, 1, 0, 0); // reset transform after arrows drawn
    // show help
    if (this.mouse.over) {
      if (this.arrows.length === 0 && !helpItemsUsed.active) {
        drawHelpText(ctx, TABLE.help.active, TABLE.image);
      } else if (this.closestArrowIndex > -1 && !helpItemsUsed.activeArrow) {
        drawHelpText(ctx, TABLE.help.activeArrow, TABLE.image);
      } else if (this.closestArrowIndex === -1 && !helpItemsUsed.closeTable) {
        drawHelpText(ctx, TABLE.help.closeTable, TABLE.image);
      }
    }
  } else {
    this.drawEmpty();
  }
}

// renders a table. Stops rendering if the mouse is not over
function tableUpdate() {
  if (this.mouse.over) {
    this.updating = true;
    requestAnimationFrame(this.update);
  } else {
    this.buttonDown = false; // turn of button if dragged off
    this.div.style.cursor = "default";
    this.updating = false;
    this.draw(); // draw another time. This alows for the visual state to be correct
  }
  this.draw();
  this.div.style.cursor = this.cursor;
}

// Mousecallback starts a table rendering if not allready doing so.
function mouseInOutCallback() {
  if (this.mouse.over) {
    if (!this.updating) {
      this.update();
    }
  } else {
    this.div.style.cursor = "default";
  }
}

function createAddTable() { // Creates a table. Tables default in inactive
  "use strict";
  var table = {},
    div = document.createElement("div"),
    canvas = document.createElement("canvas");
  div.style.width = TABLE.width + "px";
  div.style.height = TABLE.height + "px";
  div.style.display = TABLE.DOM.display;
  canvas.width = TABLE.width;
  canvas.height = TABLE.height;
  canvas.className = TABLE.DOM.tableClass;
  canvas.style.zIndex = TABLE.DOM.zIndex;
  table.div = div;
  table.canvas = canvas;
  table.ctx = canvas.getContext("2d");
  table.arrows = [];
  table.width = TABLE.width;
  table.height = TABLE.height;
  table.mouseOverClose = false;
  table.drawCloseIcon = drawClose;
  table.draw = drawTable;
  table.dragging = false;
  table.active = false;
  table.update = tableUpdate.bind(table);
  table.mouseOver = mouseInOutCallback; // called by mouseEvent when mouse over out
  table.drawEmpty = drawEmpty.bind(table);
  table.dead = false; // when removed and not needed it is dead and can then be removed from table array
  table.updating = false; // true is animation requests are happening
  div.appendChild(canvas); // add canvas
  table.mouse = createMouse(table);
  table.draw();
  return table;
}

function addTable() { // Adds a table to table array and DOM
  "use strict";
  var table = createAddTable(); // create new table
  TABLE.tables.appendChild(table.div); // add to the dom
  table.mouse.start(); // start the mouse
  tableArray.push(table); // add to table array
  return table;
}

function drawEmpty() { // draw empty table and handle click on empty table
  var ctx = this.ctx;
  ctx.drawImage(emptyTableImage, 0, 0);
  if (this.mouse.over) {
    ctx.globalCompositeOperation = "lighter";
    ctx.globalAlpha = TABLE.empty.highlightAmount;
    ctx.drawImage(emptyTableImage, 0, 0);
    ctx.globalAlpha = 1;
    ctx.globalCompositeOperation = "source-over";
    if (!helpItemsUsed.empty) { // show help is the help action has not yet been done
      drawHelpText(ctx, TABLE.help.empty, TABLE.empty);
    }
    this.cursor = TABLE.empty.cursor;
    if (this.mouse.button === 1) { // bit field
      this.buttonDown = true;
    } else if (this.buttonDown) {
      this.active = true;
      setTimeout(addTable, TABLE_REFRESH_DELAY);
      this.buttonDown = false;
      helpItemsUsed.empty = true; // flag this help as not needed as user has complete that task
    }
  } else {
    this.cursor = "default";
  }
}

// create the mouse inteface for a table
function createMouse(table) {
  var mouse = {
    x: 0,
    y: 0,
    over: false,
    table: table,
    element: table.div,
    button: 0
  };
  mouse.event = mouseEvent.bind(mouse);
  mouse.start = function() {
    MOUSE.events.forEach(n => {
      this.element.addEventListener(n, this.event);
    });
  }
  mouse.remove = function() {
    MOUSE.events.forEach(n => {
      this.element.removeEventListener(n, this.event);
    });
  }
  return mouse;
}


addTable();
body {
  background-color: #982439;
}
#table {
  padding: 10px;
}
canvas {
  position: absolute;
}
<div id="tables">
</div>

\$\endgroup\$
0

1 Answer 1

8
+100
\$\begingroup\$

This turned out to be a bit lengthy. I am not sure if I covered everything, but this should help you with cleaning up your code.

JSLint

Strings

In Javascript strings can be surrounded by both double quotes and single quotes. JSLint wants you to turn all single quoted strings into double quoted strings to make strings uniform. If you need to use a double quote in such a string, escape it (\").

Variable declaration

JSLint expects you to declare one variable per line, per var keyword. While you can chain variable declarations together with commas, doing so makes it harder to find the declaration of a particular variable, and is error prone if the declarations are split over multiple lines. Forgetting a comma will cause automatic insertion of a semicolon (ASI), which will drop the following variables in the global scope. This will produce hard to debug bugs. Modifying such a continued statement is harder, because removing or adding a variable does not only involve that variable, but also the variables around it.

Trailing space

JSLint complains about a trailing space, because it is invisible without an other character behind it. It's hardly an error, but easy to correct with a global replace in your editor/IDE.

for-loops

JSLint recommends usage of the array functions over a raw for-loop. For example, you can rewrite the following for-loop:

for (i = 0; i < len; i += 1) {
  ctx.strokeText(text[i], ctx.canvas.width / 2 + 1, yy);
  yy += TABLE.empty.font.size * 1.2;
}

to use Array.prototype.forEach(..) instead.

text.forEach(function(cur) {
  ctx.strokeText(cur, ctx.canvas.width / 2 + 1, yy);
  yy += TABLE.empty.font.size * 1.2;
});

The following for-loop is actually a complicated filter:

  for (i = 0; i < tableArray.length; i += 1) {
    if (tableArray[i].dead) {
      tableArray.splice(i, 1);
      i -= 1;
    }
  }

instead of:

  tableArray = tableArray.filter(function(cur) {
    return !cur.dead;
  });

You will see that it is much easier to see what you are doing with the for-loop when you write it with one of the array functions instead. In some cases it will only add clarity which array you are looping through, while in some cases it will clean up the code a lot and makes it trivial to verify that your code is actually doing what you expect it to do.

this

JSLint wants you to avoid this regardless if you use strict mode. this has it's uses, but you don't need to use it usually. There is this answer on SO that goes in great detail on this. Avoiding this will prevent problems where you suddenly can't reference a variable, because the code is inside a handler, causing you to have to alias this. Similarly, it avoids ambiguity in the code when reading it later, because you don't have to remember which contexts are using their own this.

In strict mode, this is... stricter. It no longer exposes the window object and no longer boxes everything into an object. You can read mdn and this answer on why it impacts anonymous functions.

The "strict violation" may not actually cause a problem when the code is run. JSLint does not have the luxery of evaluating this at run-time, and will return an error when it thinks it might cause a problem. You can read this answer about why it is illegal to use this in that context. To get rid of the error, simply avoid this altogether.

Some added explanation at the end.

Bitwise operators

The bitwise operators are rarily used in javascript, and JSLint will report usage of each and every one of them, because often usage of one is simply a typo. I am not sure why you are using e.which near that code, but it is non-standard and should not be used.

In any case, review each case of this error manually, and ignore those that actually make sense in your code.

Arrow functions

JSLint only wants arrow functions when the arguments are surounded by parenthesis, and the body of the function does not have braces around it. That means that arrow functions according to JSLint may only consist of one statement, and that it returns whatever the return value of that one statement is. If you don't care if it returns anything or not, remove the braces. Otherwise just create a regular anonymous function.

Circular references

Your problem has everything to do with circular references. The best solution is always to refactor the code in such a way that these circular references are not needed.

If you are just out to get rid of jslint errors, you can do it with forward declaration:

var a;
var b = function() {
  a();
};
a = function() {
  b();
};

Chained assignment

You have two chained assignment statements. JSLint complains about them. I am not a huge fan of them, because it adds extra work when one of the chained variables needs to have a different value.

Other issues

These are issues that JSLint does not complain about, but are things that should be improved in my opinion.

Variable names

You are using a lot of one letter variable names. Remember that by not typing out variables now will produce a headache when you read it back in 2 days time. Do yourself and other maintainers a favour and type out all the variable names.

ES6

You are using arrow functions, which means you are using ES6. Another useful thing in ES6 are the const and let keywords. Where var is subject to variable hoisting, and leaks out of loops they were defined in, let behaves a lot better and is only defined from the moment you declare it, and only lives to the end of the current block. const is used for constants. If you use ES6 features, I recommend using let everywhere you can't use const, and use const everywhere else. Instead of doing this:

// App constants all up top
var GLOBAL_SCALE = 1;
var SHOW_HELP = true; // set to false to have the help turned off
var SHADOW = 'rgba(0,0,0,0.8)';

do this:

// App constants all up top
const GLOBAL_SCALE = 1;
const SHOW_HELP = true; // set to false to have the help turned off
const SHADOW = "rgba(0,0,0,0.8)";

You can not assign a new value to constants, not even accidentally. What you assign to a constant is by default not immutable, but in case of objects you can for example freeze it.

Code between function declarations

Between createCloseImage(..) and drawArrow(..) you did hide some code. Don't do this. Instead move this code up towards the rest of the code, or into a constructor.

Classes

Speaking of which... It looks like the code was once in a class, but no longer is. I recommend encapsulating this code in a class. Your use of this makes no sense in this context, but likely used to point to your class instance. Since you use ES6, you can use the syntactic sugar added for classes. In any case, you can then add "use strict"; to the top of that class instead of in each method.

Non-standard use of MouseEvent

You are using MouseEvent.which, which is non-standard. Instead use MouseEvent.buttons if you wanted to detect simultanious pressed mouse buttons, or MouseEvent.button if you are only interested in one button. In the latter case, just check if a particular button is pressed with === instead of bitwise operators.

Magic numbers

Your moved some of your magic numbers to constants, but your code still contains a lot of numbers that make no immediate sense to me. For example, what is 223 in var scaleX = TABLE.width / 223? Or 35.25 in ctx.fillRect(35.25 * scaleX, 20 * scaleY, 152.5 * scaleX, 274 * scaleY);.

Edit: In the comments you let me know that 223 is the width of a table tennis table. Make this clear in the code too, by having a constant with such a name. Now, instead of scaleX being an operation on numbers that don't make immediate sense to me, you can write scaleX = TABLE.width / TABLE_TENNIS_TABLE_WIDTH;. Without me needing to know the dimensions of a table tennis table, the code still makes sense to me.


Some added explanation for the removal of this. A normal class looked something like this.

var PolarBear = function (name) {
  //Public data
  this.name = name;

  //Private data
  var MAX_SIZE = 300; //cm
  var size = Math.floor(Math.random() * MAX_SIZE);

  //Because "this" depends on the context
  var self = this;

  //Private function
  var growl = function () {
    console.log(self.name + " growls.");
  };

  //Public functions
  this.pet = function () {
    console.log("You decide to pet your polar bear named " + this.name);
    growl();
  };

  this.getSize = function () {
    return size;
  };
};

You'll notice that the private function does not actually use this., because this in that function actually points to the window object, because we are not in strict mode. Instead of aliasing this we could refer to name directly, but that would mean that if we changed the (public) name of our PolarBear, the private function would use the old name. All in all, it is needlessly messy and harder to write without testing. We could instead do it this way:

var PolarBear = function (name) {
  "use strict";

  //Public data
  let polarbear = {}
  polarbear.name = name;

  //Private data
  const MAX_SIZE = 300; //cm
  let size = Math.floor(Math.random() * MAX_SIZE);

  //Private function
  let growl = function () {
    console.log(polarbear.name + " growls.");
  };

  //Public functions
  polarbear.pet = function () {
    console.log("You decide to pet your polar bear named " + polarbear.name);
    growl();
  };

  polarbear.getSize = function () {
    return size;
  };

  //Public functions and data
  return polarbear;
};

Now if we test it with this, you see they work the same way.

var myPet = new PolarBear("Jack");
myPet.pet();

console.log(myPet.getSize());
myPet.name = "Elizabeth the 4th";
myPet.pet();

//These don't work, because they are private
console.log(myPet.size); //undefined
myPet.growl(); //TypeError

In the second example, instead of using this, we use one object that we return. This object polarbear is the public interface of our PolarBear class. In the end it works better, because we don't really have to worry about the actual scope of this, or if we need to replace it with an alias that is guaranteed the this we expected it to be. You can write the second example without needing to know the nuances of how this works, which makes it less error-prone to write it. You know the scope of polarbear, because you defined its scope.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for your answer! Only part of this code is mine, thats why some things are done in two different ways. The "magic numbers" are the actual sizes of a table tennis table in cm. width/223 is the scale of the current drawing. Can you give me an example on how to replace "this" in a proper way? Is it ok to contact you if I have more questions? :) \$\endgroup\$
    – Tweakimp
    Commented Aug 22, 2016 at 3:05
  • \$\begingroup\$ I have added some text to my answer. If you don't understand something I have written, try to see if googling the terms I used clarifies something. If after you have revised your code you want people to review it again, post a follow up question. This meta question explains how to do this. \$\endgroup\$
    – Sumurai8
    Commented Aug 22, 2016 at 10:00
  • \$\begingroup\$ Oh, and if you have a question about the site in general, or what you can and cannot do on the site or just want to say hi, feel free to drop by in the second monitor. \$\endgroup\$
    – Sumurai8
    Commented Aug 22, 2016 at 10:29

Your Answer

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

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