CODEWITHSUNDEEP
javascript
fev3r
fev3r

Reputation: 105

Determining factorial of a number

I'm new to javascript and I'm having a hard time figuring out how to get this factorial function to work. Here's my code now.

  var x = prompt("Enter a number"); {
    function fact(x) {
      if (x < 0) {
        return ("Enter a positive integer");
        }
      else {
        return (x * fact(x-1))
      }
    }
  }
  var result = fact(x)
  document.write("The factorial of" + x + "is" + result);

Thanks for the help!

Upvotes: 1

Views: 428

Answers (4)

gfullam
gfullam

Reputation: 12045

In addition to fixing the flawed algorithm, I recommend moving your prompt into its own function for separation of concerns.

I also like the idea of using a while statement for this as well as doing a parseInt on the input:

function fact(x) {
    while (x > 1) {
        return (x * fact(x-1));
    }
    return x;
}
function doFact() {
    var x = parseInt(prompt("Enter a positive integer"));
    if (x < 1) {
        doFact();
    } else {
        var result = fact(x);
        alert("The factorial of " + x + " is " + result);   
    }
}
doFact();

Upvotes: 0

amulya349
amulya349

Reputation: 1238

In every recursive function, there exists a stopping condition (in your case its if(x<=1)) without which, the function would go to infinite recursion. You had not added that stopping condition. Following is the working updated program:

var x = prompt("Enter a number"); {
    function fact(x) {
      if (x < 0) {
        return ("Enter a positive integer");
      }
      else if(x <=1){
        return 1;
      }
      else {
        return (x * fact(x-1))
      }
    }
  }
  var result = fact(x)
  document.write("The factorial of " + x + " is " + result);

Upvotes: 1

slebetman
slebetman

Reputation: 113964

Your definition of factorial is wrong. The traditional recursive definition of factorial is:

F(x) => x == 1 ? 1 : x * F(x-1)

Or you can use the iterative definition

F(x) => var i = 1; for (j = 1..x) i = i * j

In javascript, the recursive version would be:

function factorial (x) {
    if (x == 1) return x;
    return x * factorial(x-1);
}

The iterative version would be:

function factorial (x) {
    var result = 1;
    for (var y = 1; y <= x; y++) {
        result = result * y;
    }
    return result;
}

You can add the negative number check in the above functions. But in my opinion that would obscure the purpose of the function (which is to implement the traditional definition of factorial). A better approach is to move the negative number if() check outside of the factorial function. The if (x < 0) check has its own purpose that is separate from calculating factorials: input validation.

Upvotes: 1

Tommy
Tommy

Reputation: 620

your base case is wrong for a recursive factorial. change it to 

function fact(x) {
    if (x <= 1) {
        return 1
    }
    else {
        return (x * fact(x-1))
    }
}

Upvotes: 2

Related Questions