Calculating pair combinations in Java

Question

I have the following code to calculates and retrieves pair combinations:

public static void calculateCombinations(int x){
 int number = combination(x);
 int counter=0,y=0;
 while(counter<number){
 y++;
 for(int i=1; i<=x; i++){
 if(y!=i && !(i<y)){
 System.out.println(y + " -- " + i);
 }
 }
 counter++;
 }
 }
 public static int combination(int n)
 {
 return permutation(n) / (permutation(2) * permutation(n - 2));
 }
 public static int permutation(int i)
 {
 if (i == 1)
 {
 return 1;
 }
 return i * permutation(i - 1);
 }

The code works well but it doesn't seem an efficient and professional code. One thing I am worried about here is the running cost of this code because it is part of a very huge project, which I care about the running time. What I want is to re-write it in a very professional way. I found lot of codes that do the same job I want but I only want to write my own code. I am asking you here to suggest how I can improve this function.

Answer 1

calculateCombinations

Your method does a lot of not required iterating. You only have to iterate up to x-1 in the outer loop as afterwards the if statement evaluates always to false (-> you can remove the number variable and thus the #combination call). The value of y and counter is the same on each iteration -> you can drop the counter variable. The if statement can be removed by starting the inner loop at y+1.
The resulting code would look something like:

public static void calculateCombinations(int x) {
 int y = 0;
 while (y < x - 1) {
 y++;
 for (int i = y + 1; i <= x; i++) {
 System.out.println(y + " -- " + i);
 }
 }
}

This can be written with two for loops to keep the code shorter:

public static void calculateCombinations(int x) {
 for (int i = 1; i < x; i++)
 for (int j = i; ++j <= x;)
 System.out.println(i + " -- " + j);
}

Besides that I would rename the method as it prints the combinations, with the current name I would assume that the method returns the combinations.

permutation

This method is effectively a recursively written loop that returns the product of the range [1,i] (=factorial, thus method should be renamed to factorial) and thus can be replaced with:

public static int permutation(int i) {
 // should add check for negative values
 int p = 1;
 for (; i > 1; i--)
 p *= i;
 return p;
}

For O(1) performance you could precompute the factorials for values up to 12 (all factorials in int range).

• Peter Taylor: for (int j = i+1; etc to maintain behaviour.
• Nevay: Behavior is the same as j is pre incremented in the condition.