-1

I am trying to work out how to add 6 randomly generated numbers to a HashSet. I am getting results but they are inconsistent. Sometimes it prints 6 numbers to the console and other times it prints 5 numbers to the console.

I'm new to this stuff only this morning so I apologize if it is blatantly obvious and thank you for your help.

 HashSet<Integer> generatedLotteryNumbers = new HashSet<Integer>();
Random r = new Random();

for(int i=0; i<6; i++){
  generatedLotteryNumbers.add(r.nextInt(49));
}

System.out.println(generatedLotteryNumbers);
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
Schming
  • 51
  • 7

2 Answers2

3

This is because GeneratedLotteryNumbers is a HashSet (which acts like a set) and a HashSet in java doesn't insert an item which is already present in it and hence doesn't allow duplicates, so if you are getting less than 6 elements, that's because some elements are common, and hence stored only once.

Better try this :

ArrayList<Integer> GeneratedLotteryNumbers = new ArrayList<Integer>();
Random r = new Random();
for(int i=0; i<6; i++){
    GeneratedLotteryNumbers.add(r.nextInt(49));
}

To detect the insertion of a duplicate, capture the boolean returned by the Set::add method, TRUE is successfully added and FALSE if duplicate.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
Jarvis
  • 8,494
  • 3
  • 27
  • 58
  • Ah right. I knew that it only stored a value once but I didn't realize that it just omitted the duplicate. Thanks. – Schming Nov 29 '16 at 03:34
  • 1
    Hashset doesn't allow the insertion of duplicates, for more info, see this : http://stackoverflow.com/questions/12940663/does-adding-a-duplicate-value-to-a-hashset-hashmap-replace-the-previous-value, and please mark the answer as accepted if it resolved your issue. :) @Schming – Jarvis Nov 29 '16 at 03:36
  • 1
    @Schming It will omit the duplicate, but the `add` method will return `false` when that happens. So it's not completely silent. – 4castle Nov 29 '16 at 03:42
  • You'd do better to keep `add`ing until the size hits your target. A counted loop as you've implemented it doesn't solve OP's problem. – pjs Nov 29 '16 at 15:39
3

A Set cannot contain duplicate values, so if the generator produces the same number twice, it will be removed. Instead, you should loop based on the size of the Set (or use a List):

while (generatedLotteryNumbers.size() < 6) {
    generatedLotteryNumbers.add(r.nextInt(49));
}

If you're using Java 8, another option is to use Random#ints to generate a Stream which you can use to directly create your Set.

Set<Integer> generatedLotteryNumbers = r.ints(0, 49)
                                        .distinct()
                                        .limit(6)
                                        .boxed()
                                        .collect(Collectors.toSet());
4castle
  • 32,613
  • 11
  • 69
  • 106
  • Interesting. I didn't realize you could call `distinct()` on an infinite stream. – shmosel Nov 30 '16 at 00:40
  • A slight problem with this answer is that it can *theoretically* run infinitely, and performance is likely to degrade as the target size (6) approaches the max value (49). I think a more correct approach would be to shuffle an array of all potential values and select the first n elements. – shmosel Nov 30 '16 at 00:47
  • @schmosel Well if we're being theoretical, the probability of the loop running infinitely converges on 0 with each iteration. So theoretically, there is 0% probability of the loop being infinite. Could you explain what you mean with performance issues? – 4castle Nov 30 '16 at 01:07
  • Benchmark your code with a range of 1,000,000 and a limit of 100,000. Now change the limit to 900,000 and it'll take significantly longer. – shmosel Nov 30 '16 at 01:23
  • @shmosel Ah, I see. Well, in that case another option would be to remove a random item from the list until it reaches the desired length. I think the best algorithm is dependent on the density of the set desired from the range. I'd be interested to see what algorithms would be appropriate for each density. – 4castle Nov 30 '16 at 03:49
  • Well, create a `List` containing all choices, e.g. `List list=IntStream.rangeClosed(1, 49).boxed().collect(Collectors.toCollection(ArrayList::new));`, then, pick the fixed number of random elements like `for(int i=0; i<6; i++) picked.add(list.remove(r.nextInt(list.size())));`. Of course, if the number of elements to pick is large, you can use `Collections.shuffle`, followed by `subList` taking the first *n* elements. From this you can derive [this answer](http://stackoverflow.com/a/28671411/2711488), which will handle all cases well, though, there might be special versions for `int`s… – Holger Dec 05 '16 at 18:08