Generating permutations and combinations

How can this code be improved?

```
using System;
//using System.Linq;
using System.Collections;
using System.Collections.Generic;

namespace Combinatorics
{
public class Permutation
{
private int[] data = null;
private int order = 0;
public Permutation(int n)
{
this.data = new int[n];
for (int i = 0; i = 0; --i)
{
this.data[i] = temp[i];
for (int j = i + 1; j = this.data[i])
++this.data[j];
}
}
for (int i = 0; i result.data[left + 1]) && (left >= 1))
{
--left;
}
if ((left == 0) && (this.data[left] > this.data[left + 1]))
return null;

right = result.order - 1; // Step #2 - find right; first value > left
while (result.data[left] > result.data[right])
{
--right;
}

int temp = result.data[left]; // Step #3 - swap [left] and [right]
result.data[left] = result.data[right];
result.data[right] = temp;

int i = left + 1; // Step #4 - order the tail
int j = result.order - 1;

while (i tempList = new List(data);

for (int i = 0; i this.n - 1)
return false; // value out of range

for (int j = i + 1; j = this.data[j])
return false; // duplicate or not lexicographic
}

return true;
} // IsValid()
public Combination Next()
{
if (this.data[0] == this.n - this.k)
return null;

Combination ans = new Combination(this.n, this.k);

int i;
for (i = 0; i 0 && ans.data[i] == this.n - this.k + i; --i)
;

++ans.data[i];

for (int j = i; j x)
--v;

return v;
} // LargestV()
public static ArrayList Generate(Object[] list, int choose, bool random)
{
Combination c = new Combination(list.Length, choose);
ArrayList result = new

-
Why have you commented out Linq namespace? Use it at least here:

this.data = new int[n];
for (int i = 0; i < n; ++i)
{
    this.data[i] = i;
}

It can be replaced with:

this.data = Enumerable.Range(0, n).ToArray();

-
In first two Permutation constructors you have:

public Permutation(int n)
{
    this.data = new int[n];
    ...
    this.order = n;
}

public Permutation(int n, int k)
{
    this.data = new int[n];
    this.order = this.data.Length;
    ...
}

Effectively, order is being set to the same value, but you're doing it in a different way in similar scenarios. This makes it more confusing. Either use order = n in both places or use order = data.Length. Keep it consistent.

-
In the Permutation class, you have field order which is always set to be a length of data and you also have a property to retrieve this field. I believe this additional field makes this code more error prone because you have to remember to set this field. I would remove it and replace Order property to return data.Length.

-
In many places, it is unclear what's going on and how to use these classes. Add some documentation. I would never guess how Permutation.Element(n) would return new Permutation.

-
This statement in the second Permutation constructor is misleading:

for (int i = 0; i < n; ++i)
{
    temp[i] = ++factoradic[i]; // <-- Why ++  ???
}

I would remove ++ here. In this way it makes me think that factoradic[i] is used somewhere else, but it is not. Use factoradic[i] + 1 instead. Remember that when somebody will read/review your code he will have to keep in mind all the variables in method. This line just made this reviewer to update variable value which is not a cheap operation for people.

Also consider using LINQ here and replace it with:

int[] temp = factoradiac.Select(i => i + 1).ToArray();

I wasn't strong enough to read through second class, so probably some my points will be applicable there also.

StackExchange Code Review Q#1352, answer score: 4