patternjavascriptMinor
Table tennis exercise builder
Viewed 0 times
tennisexercisetablebuilder
Problem
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:
`// 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,
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";andthisin the same function gives a "strict violation". I would like to use"use strict";, but does that mean everythishas 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,
Solution
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:
to use
The following for-loop is actually a complicated filter:
instead of:
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
In strict mode,
The "strict violation" may not actually cause a problem when the code is run. JSLint does not have the luxery of evaluating
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
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:
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
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
Code Snippets
for (i = 0; i < len; i += 1) {
ctx.strokeText(text[i], ctx.canvas.width / 2 + 1, yy);
yy += TABLE.empty.font.size * 1.2;
}text.forEach(function(cur) {
ctx.strokeText(cur, ctx.canvas.width / 2 + 1, yy);
yy += TABLE.empty.font.size * 1.2;
});for (i = 0; i < tableArray.length; i += 1) {
if (tableArray[i].dead) {
tableArray.splice(i, 1);
i -= 1;
}
}tableArray = tableArray.filter(function(cur) {
return !cur.dead;
});var a;
var b = function() {
a();
};
a = function() {
b();
};Context
StackExchange Code Review Q#139109, answer score: 8
Revisions (0)
No revisions yet.