# Find all the positive divisors of a positive integer

This came from this question.

Find all the positive divisors of an integer >= 2.

Can stop when i * i >= number.

Please review for speed and style.

public class IntWithDivisors
{
public override int GetHashCode()
{
return Number;
}
public override bool Equals(object obj)
{
if(obj is IntWithDivisors)
{
return ((IntWithDivisors)obj).Number == this.Number;
}
return false;
}
public override string ToString()
{
return $"number {Number} Devisors " + string.Join(", ", Divisors); } public List<int> Divisors { get; } = new List<int>(); public int Count { get { return Divisors.Count(); } } public int Number { get; } public IntWithDivisors(int number) { if (number < 2) { throw new ArgumentOutOfRangeException(); } Number = number; Divisors = IntDivisors(number); } } public static List<int> IntDivisors(int num) { //Debug.WriteLine($"\nIntDevisors  {num}");
List<int> intDivisors = new List<int>();
int i;
int incr;
if(num / 2 * 2 == num)
{
i = 2;
incr = 1;
}
else
{
i = 3;
incr = 2;
}
for(; i*i < num; i += incr)
{
int numOveri = num / i;
if (numOveri * i == num)
{
//Debug.WriteLine(i);
}
}
if(i*i == num)
{
}
intDivisors.Sort();
return intDivisors;
}


Why is the implementation of the algorithm a static method outside of the class IntWithDivisors? Is it in class Program? It shouldn't be there.

It is unusual to do complex work in the constructor. A constructor should only perform initialization. See answer of Telastyn in how complex a constructor should be. There are different possibilities to solve this

E.g. lazy evaluation:

private List<int> _divisors;
public List<int> Divisors
{
get {
if (_divisors == null) {
_divisors = IntDivisors(Number);
}
return _divisors;
}
}


Another option is to simply call a method to get the result.

This leads us to the next question: what is the task of the class IntWithDivisors? Do we really need to store the input number together with the output? Do we really need to override Equals and GetHashCode?

I would rather opt for a minimalist but reusable and flexible approach in the LINQ style as extension method implemented as iterator.

I observed that the starting i is one bigger than incr. We can use this fact to simplify the initialization.

Testing if a number is even is usually done with the modulo operator which yields the remainder of the division num % 2 == 0.

numOveri is a strange name. I renamed it to quotient and i to divisor.

The divisors are tested in ascending order; however, the quotients accumulate in descending order. Therefore, we can return the divisors immediately with yield return and store the quotients in a list. We then need to reverse this list before we return its items.

public static class IntExtensions
{
public static IEnumerable<int> SelectDivisors(this int num)
{
yield return 1;

int incr = num % 2 == 0 ? 1 : 2;
var largeDivisors = new List<int>();
for (int divisor = incr + 1; divisor * divisor <= num; divisor += incr) {
int quotient = num / divisor;
if (quotient * divisor == num) {
yield return divisor;
if (quotient != divisor) {
}
}
}

largeDivisors.Reverse();
for (int k = 0; k < largeDivisors.Count; k++) {
yield return largeDivisors[k];
}

yield return num;
}
}


We can use this extension method like this (shown in a little test routine):

int[] numbers = new[] { 9, 12, 15, 16, 17, 27, 54 };
foreach (int num in numbers) {
Console.Write(\$"Number {num} has divisors ");
foreach (int n in num.SelectDivisors()) {
Console.Write(n + " ");
}
Console.WriteLine();
}

• Thanks. My though on public static List<int> IntDivisors is so that it could be called directly. You want divisors on a single number. If a class can define gethashcode and equals then I always implement. I like the better names. Quotient is not really a good name. It is the BigDivisor. So maybe DivisorSmall and DivisorBig. In my testing n / i * i was faster then modus. Jun 2, 2018 at 16:58
• I like IntExtensions and the reverse to avoid a sort. Jun 2, 2018 at 17:04
• Well, the quotient can be a divisor or not, depending on whether there is a remainder. But these namings are always a matter of taste. As long as they are understandable, they're all okay. Jun 2, 2018 at 17:16
• Not arguing. But divisor can also be a divisor or not, depending on whether there is a remainder. Great feedback. Jun 2, 2018 at 17:44

What I ended up with

public static class IntExtensions
{
public static IEnumerable<int> SelectDivisors(this int num)
{
yield return 1;

int incr = (num / 2 * 2 == num) ? 1 : 2;
var largeDivisors = new List<int>();
for (int smallDivisor = incr + 1; smallDivisor * smallDivisor <= num; smallDivisor += incr)
{
int largeDivisor = num / smallDivisor;
if (largeDivisor * smallDivisor == num)
{
yield return smallDivisor;
if (largeDivisor != smallDivisor)
{
}
}
}

for (int k = largeDivisors.Count - 1; k >= 0; k--)
{
yield return largeDivisors[k];
}

yield return num;
}
}

• Why would posting what I ended up with get down votes? Jun 3, 2018 at 16:47
• Self answers should be answers as well. This is just a code dump without a review of the code. If you can't add anything new you should refuse self-answers and better ask a follow-up question. Jun 4, 2018 at 4:31