0

I'm trying to write a method that returns one large ArrayList<ArrayList<String>> that contains smaller ArrayLists that each have a different permutation of a starting ArrayList.

This is my method: 

public static ArrayList<ArrayList<String>> permute(ArrayList<String> x) {

    ArrayList<ArrayList<String>> res = new ArrayList<ArrayList<String>>();

    while (res.size() < fac(x.size())) {  //fac method works fine

        Collections.shuffle(x);

        if (!res.containsAll(x)) {
            res.add(x);
        }

    }

    return res;
}

My approach is to basically keep reshuffling the original ArrayList, x, and check if its already in the resultant ArrayList, if it's not, then I add it. For some reason, when I try this method, the resultant ArrayList contains the same ArrayLists, even though I have an if statement which is there specifically so that wouldn't happen.

What am I missing?

ninesalt
  • 4,054
  • 5
  • 35
  • 75
  • 3
    There are better ways to calculate permutations than shuffle since this can be really bad for your performance. Moreover res is an List of ArrayList whereas x is a List of String so you can t compare them with contains all – PKuhn Sep 07 '15 at 18:36
  • You can get hints for more efficient solutions [here](http://rosettacode.org/wiki/Permutations#Java) and [here](http://stackoverflow.com/a/14444037/1413133) – Manos Nikolaidis Sep 07 '15 at 18:47

2 Answers2

6

There are three problems with your algorithm:

  1. You are re-shuffling and adding the same list again and again. Try adding a x = new ArrayList(x) somewhere in the loop.
  2. As pointed out by Manos, you have to use contains, not containsAll; otherwise you are checking whether all the elements inside the newly shuffled array list are contained, which is never the case, thus you are adding the same list again.
  3. Your algorithms is horribly, horribly slow. Once you've got the above two problems worked out, so that the algorithm would in priciple work, the probability to get just the last permutation will be 1/n! (for n elements), so this will take really, really long.
tobias_k
  • 81,265
  • 12
  • 120
  • 179
  • I understand the n!, but why is it 1/n! ? – ninesalt Sep 11 '15 at 19:12
  • @Swailem95 Because you are rolling random numbers. Once you've got all but one of the `n!` permutations, the probability for getting that final one out of the total `n!` that could possibly come up is `1/n!` – tobias_k Sep 11 '15 at 20:02
0

res.containsAll(x)

This will be true if all elements of type String in x exist in res. But the elements of res are ArrayList<String>. You probably want :

res.contains(x)

This will be true if x which is a ArrayList<String> exists inside res.

Manos Nikolaidis
  • 21,608
  • 12
  • 74
  • 82
  • I tried that. It just goes on an infinite loop because the while loop condition is never reached for some reason. Res' size is always 1, but the ArrayList inside is keeps changing. – ninesalt Sep 07 '15 at 18:41
  • 1
    @Swailem95 That's because you keep shuffling the same collection. You need to create a new collection and shuffle it. – RealSkeptic Sep 07 '15 at 18:42