Factorial Function - Design and Testing

Question

I'm trying to nail a few interview questions, so I stared at the simple one.

Create a factorial function.

This function is a worksheet (no dependencies — easily verifiable), so I made it static inside the helper class.

public static class MathHelper
{
    public static int Factorial(int n)
    {
        Debug.Assert(n >= 0);
        if (n < 0)
        {
            throw new ArgumentException("n cannot be lower that 0");
        }

        Debug.Assert(n <= 12);
        if (n > 12)
        {
            throw new OverflowException("Overflow occurs above 12 factorial");
        }

        int factorialOfN = 1;
        for (int i = 1; i <= n; ++i)
        {
            //checked
            //{
                factorialOfN *= i;   
            //}
        }
        return factorialOfN;
    }
}

Testing:

    [TestMethod]
    [ExpectedException(typeof(OverflowException))]
    public void Overflow()
    {
      int temp = FactorialHelper.MathHelper.Factorial(40);
    }

    [TestMethod]
    public void ZeroTest()
    {
        int factorialOfZero = FactorialHelper.MathHelper.Factorial(0);
        Assert.AreEqual(1, factorialOfZero);
    }

    [TestMethod]
    public void FactorialOf5()
    {
       int factOf5 = FactorialHelper.MathHelper.Factorial(5);
       Assert.AreEqual(5*4*3*2*1,factOf5);
    }

    [TestMethod]
    [ExpectedException(typeof(ArgumentException))]
    public void NegativeTest()
    {
        int factOfMinus5 = FactorialHelper.MathHelper.Factorial(-5);
    }

I have a few questions:

• Is it correct? (I hope so;))
• Throws the correct exceptions?
• Should I use a tested context or is this trick (n> 12) ok?
• Is it better to use uint istead to check for negative values?
• Future improvement: overload for long, decimal, BigInteger, or perhaps a common method?

thank

Answer 1

, . , , ( ) , . :

bigint myFactorial(uint first, uint last)
{
    if (first == last) return first;
    uint mid = first + (last - first)/2;
    return myFactorial(first,mid) * myFactorial(1+mid,last);
}
bigint factorial(uint n)
{
    return myFactorial(2,n);
}

, :

• ( )

Answer 2

• , .
• , , , .
• . , , 12 .
• Uint. - , .
• ( bigint) (int - )

, , :

• ? - .
• , 12! 11!?
• n==0 ?

, , , . , , .

Answer 3

In a for loop, you can start with (int i = 2 ...). Multiplying by 1 is completely useless. I would choose one ArgumentOutOfRangeException for both <0 and> 12. Debug.Assert will mask the exception when you use unit test (you will have to test it in Release mode).