Most efficient way to write Combination and Permutation calculator in Javascript
Well, here we go!
First of all, why would you ever need to write this?
Math.divide = function(a,b)
{
return a/b;
}
I would do away with it completely.
You can also clean up your Math.factorial
a little bit:
Math.factorial = function(n)
{
n = Number(n);
if (isNAN(n)) {
alert("Factorial requires a numeric argument.");
return null;
} else if (n < 2) {
return 1;
} else {
return (n * Math.factorial(n - 1));
}
}
But the main problem is your onclick()
code:
onclick="var n = T1.value; var r = T2.value; var n_minus_r = parseFloat(n) - parseFloat(r); var numerator = Math.factorial(T1.value); var n_minus_r_fact = Math.factorial(n_minus_r); var r_fact = Math.factorial(r); var denominator = n_minus_r_fact * r_fact; T3.value = Math.divide(numerator,denominator); return true;
This is way too complicated. I'd make it a function and bind it to the element, which would get rid of all of the crap in your HTML and make it a bit easier to work with:
window.onload = function()
{
document.getElementById('calculate').onclick = function() {
var n = T1.value,
r = T2.value;
T3.value = Math.factorial(n) / (Math.factorial(r) * Math.factorial(n - r));
}
}
And just get rid of the onclick=
code.
If you're concerned about efficiency, you'd probably want to re-implement the factorial as an iterative function rather than a recursive one. The recursive version will use a lot more memory and CPU time than the iterative version.
function factorial(n) {
var x=1;
var f=1;
while (x<=n) {
f*=x; x++;
}
return f;
}
You also shouldn't be adding your own functions to the Math namespace. It's not a good habit to get into.