admin管理员组

文章数量:1025457

I am trying this simple code to calculate factorial of 5. But I am getting "undefined" as the result. I am aware of other methods but what is wrong with this?

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title> Learning </title>
<head>
<body>
<h2> Wele<h2>
<p id="demo"></p>
<script>
var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   calfact(num);
  }
 else
  {
   return fact;
  }
}

document.getElementById("demo").innerHTML=calfact(5);
</script>
</body>
</html>         

I am trying this simple code to calculate factorial of 5. But I am getting "undefined" as the result. I am aware of other methods but what is wrong with this?

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title> Learning </title>
<head>
<body>
<h2> Wele<h2>
<p id="demo"></p>
<script>
var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   calfact(num);
  }
 else
  {
   return fact;
  }
}

document.getElementById("demo").innerHTML=calfact(5);
</script>
</body>
</html>         
Share Improve this question asked Apr 15, 2017 at 13:42 Shivam MishraShivam Mishra 1,4492 gold badges12 silver badges30 bronze badges 2
  • 2 if (num!=1) the function returns nothing (undefined) – Jonas Wilms Commented Apr 15, 2017 at 13:43
  • Possible duplicate of Fast factorial function in JavaScript – Krisztián Balla Commented Sep 15, 2018 at 13:02
Add a ment  | 

3 Answers 3

Reset to default 3

If you want a result from a recursive function, all code paths through the function must return something. Your code isn't returning anything in the num!=1 case. It should be returning the result of calling itself, e.g. (see the *** line):

var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   return calfact(num); // ***
  }
 else
  {
   return fact;
  }
}

Your function is using a global variable, which isn't a great idea as it means the funtion isn't self-contained; and isn't a true factorial function, because you're effectively using two inputs (fact — the global  and num, the argument)

If you want a true factorial, you don't need a global variable, just work from the argument itself:

function factorial(num) {
    if (num < 0) {
        throw new Error("num must not be negative");
    }
    if (num <= 1) {
        // Both 1! and 0! are defined as 1
        return 1;
    }
    return num * factorial(num - 1);
}
console.log(factorial(5)); // 120

Or of course, more pactly:

function factorial(num) {
    if (num < 0) {
        throw new Error("num must not be negative");
    }
    return num <= 1 ? 1 : num * factorial(num - 1);
}

(More about 0!: https://en.wikipedia/wiki/Factorial)

var fact=5;
function calfact(num){
   if(num!=1){
      fact=fact*(num-1);
      num=num-1;
      return calfact(num);//the missing thing
   }else{
      return fact;//why fact? i think it should be 1
   }
 }

By the way, your approach is maybe working, but really bad style.May do this:

function calfact(num){
  if(num!=1){
    return calfact(num-1)*num;
  }else{
    return 1;
 }
}

Or short:

calfact=num=>num==1?1:calfact(num-1)*num;

You can use Tail Recursion, which is more efficient in case of memory.

const factorial = (n, acc = 1) => n == 0 || n == 1 ? acc : factorial(n - 1, acc * n);

console.log(factorial(10))

I am trying this simple code to calculate factorial of 5. But I am getting "undefined" as the result. I am aware of other methods but what is wrong with this?

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title> Learning </title>
<head>
<body>
<h2> Wele<h2>
<p id="demo"></p>
<script>
var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   calfact(num);
  }
 else
  {
   return fact;
  }
}

document.getElementById("demo").innerHTML=calfact(5);
</script>
</body>
</html>         

I am trying this simple code to calculate factorial of 5. But I am getting "undefined" as the result. I am aware of other methods but what is wrong with this?

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title> Learning </title>
<head>
<body>
<h2> Wele<h2>
<p id="demo"></p>
<script>
var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   calfact(num);
  }
 else
  {
   return fact;
  }
}

document.getElementById("demo").innerHTML=calfact(5);
</script>
</body>
</html>         
Share Improve this question asked Apr 15, 2017 at 13:42 Shivam MishraShivam Mishra 1,4492 gold badges12 silver badges30 bronze badges 2
  • 2 if (num!=1) the function returns nothing (undefined) – Jonas Wilms Commented Apr 15, 2017 at 13:43
  • Possible duplicate of Fast factorial function in JavaScript – Krisztián Balla Commented Sep 15, 2018 at 13:02
Add a ment  | 

3 Answers 3

Reset to default 3

If you want a result from a recursive function, all code paths through the function must return something. Your code isn't returning anything in the num!=1 case. It should be returning the result of calling itself, e.g. (see the *** line):

var fact=5;
function calfact(num)
{
 if(num!=1)
  {
   fact=fact*(num-1);
   num=num-1;
   return calfact(num); // ***
  }
 else
  {
   return fact;
  }
}

Your function is using a global variable, which isn't a great idea as it means the funtion isn't self-contained; and isn't a true factorial function, because you're effectively using two inputs (fact — the global  and num, the argument)

If you want a true factorial, you don't need a global variable, just work from the argument itself:

function factorial(num) {
    if (num < 0) {
        throw new Error("num must not be negative");
    }
    if (num <= 1) {
        // Both 1! and 0! are defined as 1
        return 1;
    }
    return num * factorial(num - 1);
}
console.log(factorial(5)); // 120

Or of course, more pactly:

function factorial(num) {
    if (num < 0) {
        throw new Error("num must not be negative");
    }
    return num <= 1 ? 1 : num * factorial(num - 1);
}

(More about 0!: https://en.wikipedia/wiki/Factorial)

var fact=5;
function calfact(num){
   if(num!=1){
      fact=fact*(num-1);
      num=num-1;
      return calfact(num);//the missing thing
   }else{
      return fact;//why fact? i think it should be 1
   }
 }

By the way, your approach is maybe working, but really bad style.May do this:

function calfact(num){
  if(num!=1){
    return calfact(num-1)*num;
  }else{
    return 1;
 }
}

Or short:

calfact=num=>num==1?1:calfact(num-1)*num;

You can use Tail Recursion, which is more efficient in case of memory.

const factorial = (n, acc = 1) => n == 0 || n == 1 ? acc : factorial(n - 1, acc * n);

console.log(factorial(10))

本文标签: javascript factorial with recursionStack Overflow