1

I have written a method to shuffle a String array So the task is to implement WhiteElephant concept for a given string array of list of names.Should generate assignments to match the original elements. I have written method to pick a random number and used a map to store the values so that each value will have a different index. But this prints out only 5 values. and i am confused now. 

public static String[] generateAssignments(final String[] participants) {

    Random r = new Random();
    int size = participants.length;
    HashMap val = new HashMap();
    int change = 0;
    String[] assignments = new String[6];
    System.out.println("Using arrays");


    for(int i=0; i<size;i++){
        for(int j =0; j<size; j++){
            change = r.nextInt(size);
            if(val.containsValue(change) || change==i){
                continue;
            }
            else val.put(i, change);
            assignments[i] = participants[change];
            System.out.println(assignments[i]);
            break;
        }   

    }

    return assignments;
}

I appreciate your inputs. Thanks, Lucky

user2988851
  • 157
  • 2
  • 13
  • This is not a simple question. To determine if a shuffling algorithm is fair you must determine that it has an equal probability distribution over `n!` – Kon Jun 25 '14 at 22:32
  • The shuffling code will never move the first element. The test, as you have it written, passes if, and only if, the last element of the shuffled array is different from the last element of the unshuffled array. – biggusjimmus Jun 25 '14 at 22:33
  • 2
    Is there some reason you don't want to just use Collections.shuffle()? – biggusjimmus Jun 25 '14 at 22:35
  • 1
    It seems there are some other issues with your code, but aside from that you don't want a unit test to be non-deterministic. So you would want to mock out the `Random` to return a known set of results (i.e., not random) for the test. – David Conrad Jun 25 '14 at 22:37
  • 1
    @DavidConrad No need for that; you just pass in a `Random` and use a constant seed for the unit test. – chrylis -cautiouslyoptimistic- Jun 25 '14 at 22:43
  • @chrylis good point, although the OP's code creates a `new Random`. – David Conrad Jun 25 '14 at 23:06
  • Is it justified to create a question for "how do I test 'X'"? Shouldn't the same guidelines apply to any code? – Alexandre Santos Jun 25 '14 at 23:09
  • @AlexandreSantos I see what you are saying and I would usually agree. However I feel this might be the exception to the rule since testing code dependent on Random data is often very hard. Even seeded randomness doesn't actually prove randomness. – Jesse Webb Jun 25 '14 at 23:11

4 Answers4

2

If your shuffle method is random (or pseudorandom) it will be near impossible to unit test since the output is non deterministic. If you allow for seeding a random number generator then you could ensure the output is consistent given the same seeds, though this doesn't show randomness.

You could also run the shuffle method a large number of times and check that each card shows up at each index an approixmately equal number of times. Over a large enough number of simulations this should help illustrate randomness.

Jeff Storey
  • 56,312
  • 72
  • 233
  • 406
1

FYI - There are some logical errors in both your shuffle() code and the test. I won't address those here; hopefully having a good test will allow you to figure out the problems!

Writing tests around Random data is hard.

The best option is to pass in an instance of Random to your shuffle() method or it's containing class. Then in test usages, you can pass in an instance of Random which has been seeded with a known value. Given that the Random code will behave the same every time and you control the input array, your test will be deterministic; you can confidently assert on each object in the sorted collection.

The only downside of this approach is that you won't have a test preventing you from re-writing your shuffle() method to simply re-order the elements every time into this specified order. But that might be over-thinking it; usually we can trust our future selves.

An alternative approach is to assume that in a Random world, given enough time, every data possibility will be realized.

I used this approach when testing a 6-sided die's roll() method. I needed to ensure that getting all values from 1-6 was possible. I didn't want to complicate the method signature or the Die constructor to take in an instance of Random. I also didn't feel confident in a test that used a known seed and simply always asserted 3 (i.e.).

Instead, I made the assumption that given enough rolls, all values from 1-6 would eventually be rolled. I wrote a test that infinitely called roll until all values from 1-6 had been returned. Then I added a timeout to the test so that it would fail after 1 second if the above condition hadn't been met.

@Test(timeout = 1000)
public void roll_shouldEventuallyReturnAllNumbersBetweenOneAndSixInclusively() {
    Die die = new Die();
    Set<Integer> rolledValues = new HashSet<Integer>();
    int totalOfUniqueRolls = 0;
    while (rolledValues.size() < Die.NUM_SIDES) {
        if (rolledValues.add(die.roll())) {
            totalOfUniqueRolls += die.getFaceValue();
        }
    }
    assertEquals(summation(1, Die.NUM_SIDES), totalOfUniqueRolls);
}

Worst case scenario it fails after 1 second (which hasn't happened yet) but it usually passes in about 20 milliseconds.

Jesse Webb
  • 43,135
  • 27
  • 106
  • 143
1

The test must be reproducible: it is not useful if it depends on something random.

I suggest you to use mocking so the CUT (code under test) don't use the real Random class instantiated in production, but a different class written by you with a predictable behavior, giving you the possibility to make some significant assertions on two or three items.

marcello
  • 146
  • 1
  • 5
0

It appears your shuffle() method will always return the same result. So given a input test array of however many elements in your test, just specify the exact output array you'd expect.

It looks like you are trying to write a very general test. Instead, your tests should be very specific: given a specific input A then you expect a specific output B.

Rodney Gitzel
  • 2,652
  • 16
  • 23
  • The *current* implementation of `shuffle()` guarantees predictable output based on the input but I would argue that a `shuffle()` method is not guaranteed to have that behavior. If I change the implementation of `shuffle()`, the type of test you're suggesting will break. – Jesse Webb Jun 25 '14 at 22:44