gotchajavascriptMinor
JSLint says "unexpected function" doCalc
Viewed 0 times
jslintfunctiondocalcunexpectedsays
Problem
See the full calculator with HTML -> https://gist.github.com/1861120
You should be able to drag that html file from github into a browser and start using it. I'm only putting the js below. The calculator works fine but JS Lint is telling me I have an unexpected 'function' doCalc line 9 character 1.
Also just looking for some general tips on best practices and how this might be refactored. Any help is appreciated. Thank you.
You should be able to drag that html file from github into a browser and start using it. I'm only putting the js below. The calculator works fine but JS Lint is telling me I have an unexpected 'function' doCalc line 9 character 1.
Also just looking for some general tips on best practices and how this might be refactored. Any help is appreciated. Thank you.
var type, n, r, formula;
Math.factorial = function (n) {
if (n < 2) {
return 1;
}
return (n * Math.factorial(n - 1));
}
function doCalc(type) {
switch (type) {
case 'permutation':
n = document.permutation.T1.value;
r = document.permutation.T2.value;
formula = function() {
document.permutation.T3.value = Math.factorial(n) / Math.factorial(n - r);
}
break;
case 'combination':
n = document.combination.T1.value;
r = document.combination.T2.value;
formula = function() {
document.combination.T3.value = Math.factorial(n) / (Math.factorial(r) * Math.factorial(n - r));
}
break;
}
n = Number(n);
r = Number(r);
if (isNaN(n) || isNaN(r)) {
alert("Factorial requires a numberic argument.");
return null;
}
formula();
}
window.onload = function() {
document.getElementById('calculate-p').onclick = function() {
doCalc('permutation');
}
document.getElementById('calculate-c').onclick = function() {
doCalc('combination');
}
}Solution
Well, you asked for general tips, so here goes ;)
I would almost always refrain from extending built-in types. I will "shim" to provide standard-compliant functions which are not available (e.g. "forEach" on Array.prototype), but adding new methods to existing types is a recipe for problems. You could simply have a
In fact, you could do yourself a favor and create your own
Then
I would almost always refrain from extending built-in types. I will "shim" to provide standard-compliant functions which are not available (e.g. "forEach" on Array.prototype), but adding new methods to existing types is a recipe for problems. You could simply have a
factorial function without the Math and it would work just as well.In fact, you could do yourself a favor and create your own
Math object (named something different, of course) and add your formula functions to that, so:var MathFactor = new function () {
this.factorial = function (n) {
return n < 2 ? 1 : n * this.factorial(n - 1);
};
this.permutate = function (n, r) {
return this.factorial(n) / this.factorial(n - r);
};
this.combine = function (n, r) {
return this.factorial(n) / (this.factorial(r) * this.factorial(n - r));
};
};Then
doCalc is just a matter of a simple numeric check, then setting your T3.value to the output of a call to MathFactor.permutate() or MathFactor.combine(). Hope that helps!Code Snippets
var MathFactor = new function () {
this.factorial = function (n) {
return n < 2 ? 1 : n * this.factorial(n - 1);
};
this.permutate = function (n, r) {
return this.factorial(n) / this.factorial(n - r);
};
this.combine = function (n, r) {
return this.factorial(n) / (this.factorial(r) * this.factorial(n - r));
};
};Context
StackExchange Code Review Q#12076, answer score: 3
Revisions (0)
No revisions yet.