Reputation: 12253
I'm trying to calculate the number of possible combinations so I'm using some maths here (to be precise factorials). For example, if I have 50 numbers and I want to organize them into groups of 5, how many groups (combinations) are possible to make. I'm using this formula: allNumbers! / (allNumbers - PerGroup)!
, but it comes up with an error for this particular example. It says that dividing by zero is forbidden. How can I manage this to work?
This is my code:
int b = 1;
int n = 1;
if (allNumbers - PerGroup == 0)
{
return 1;
}
else if (allNumbers - PerGroup == 1)
{
return allNumbers;
}
else
{
for (int i = 1; i <= allNumbers; i++)
{
b *= i;
}
for (int i = 1; i <= allNumbers - PerGroup; i++)
{
n *= i;
}
if (Enumerable.Range(1,int.MaxValue).Contains(b/n)) //line with ERROR!
{
return b/n;
}
else
{
return int.MaxValue;
}
}
Upvotes: 2
Views: 1651
Reputation: 19149
I guess the error is OutOfMemoryException because you are creating a huge amount of unnecessary integers
. (Enumerable.Range(1,int.MaxValue)
) note that each int
takes 4
bytes from your memory.
Im not sure what you are trying to do there but you can use double
type so if the number becomes very large it will just give you PositiveInfinity
.
Or you can use checked
to control integer overflow with try and catch.
Another way is to pre calculate the number when integer overflow happens. for example factorial of 14 will overflow for int
and factorial of 22 will overflow for long
.
Also you dont have to write for loop twice. you can use method for this purpose.
You dont need to check for allNumbers - PerGroup == 0
to prevent zero division. that wont happen because factorial of 0 is 1 and our factorial implementation returns 1 by its nature when the input is 0! (because the for loop never iterates in that case and the counter starts from 1.)
private static int Cominations(int allNumbers, int perGroup)
{
if(allNumbers > 13)
{
Console.WriteLine("Too big number!");
return -1;
}
return Factorial(allNumbers)/Factorial(allNumbers - perGroup);
}
private static int Factorial(int number)
{
int n = 1;
for (int i = 1; i < number; i++)
{
n *= i;
}
return n; // returns 1 when number is 0
}
If you want to calculate factorial of bigger numbers use BigInteger
type from System.Numberics namespace.
using System.Numerics;
//...
private static BigInteger Factorial(int number)
{
BigInteger n = 1;
for (int i = 1; i < number; i++)
{
n *= i;
}
return n;
}
Upvotes: 0
Reputation: 3686
Enumerable.Range(1,int.MaxValue).Contains(b/n)
check doesn't check if the value is valid, because b/n is already computed and stored as int by this time.
You get division by zero because variable n is overflowed and becomes zero. In the following code you can see how overflow occurs.
using System;
public class Test
{
public static void Main()
{
int n = 1;
for (int i = 1; i <= 50; i++) {
n *= i;
Console.WriteLine("i = {0}, n = {1}", i, n);
}
}
}
Output:
i = 1, n = 1
i = 2, n = 2
i = 3, n = 6
i = 4, n = 24
i = 5, n = 120
i = 6, n = 720
i = 7, n = 5040
i = 8, n = 40320
i = 9, n = 362880
i = 10, n = 3628800
i = 11, n = 39916800
i = 12, n = 479001600
i = 13, n = 1932053504
i = 14, n = 1278945280
i = 15, n = 2004310016
i = 16, n = 2004189184
i = 17, n = -288522240
i = 18, n = -898433024
i = 19, n = 109641728
i = 20, n = -2102132736
i = 21, n = -1195114496
i = 22, n = -522715136
i = 23, n = 862453760
i = 24, n = -775946240
i = 25, n = 2076180480
i = 26, n = -1853882368
i = 27, n = 1484783616
i = 28, n = -1375731712
i = 29, n = -1241513984
i = 30, n = 1409286144
i = 31, n = 738197504
i = 32, n = -2147483648
i = 33, n = -2147483648
i = 34, n = 0
i = 35, n = 0
i = 36, n = 0
i = 37, n = 0
i = 38, n = 0
i = 39, n = 0
i = 40, n = 0
i = 41, n = 0
i = 42, n = 0
i = 43, n = 0
i = 44, n = 0
i = 45, n = 0
i = 46, n = 0
i = 47, n = 0
i = 48, n = 0
i = 49, n = 0
i = 50, n = 0
Upvotes: 2
Reputation: 746
since allNumbers!
always contains (allNumbers - PerGroup)!
, why don't you exclude them from start.
int b = 1;
if (allNumbers - PerGroup == 0)
{
return 1;
}
else if (allNumbers - PerGroup == 1)
{
return allNumbers;
}
else
{
for (int i = (allNumbers - PerGroup + 1); i <= allNumbers; i++)
{
b *= i;
}
return b;
}
Upvotes: 1