1

I am generating an array of 6 ints within a range of 1- 54 (similar to lotto numbers). However, when trying to detect duplicates, my code is not executing. I tried to debug by simply printing "WE HAVE FOUND A DUPLICATE" if my conditional logic would catch a duplicate. Of course, it doesn't print the desired output:

18-33-39-41-41-45

I want to detect the duplicate so that I can generate another random number that is not equal to the discovered duplicate. I don't want to just increment/decrement the duplicate's value just to get another number, I want to actually generate another value between 1&54 that is not equal to the duplicate.

Here is my code:

public class Main {

    public static void main(String[] args) {

        Random rand = new Random();
        int[] luckyArray = new int[6];

        //build array
        for (int i = 0; i < luckyArray.length; i++) {

            int luckyNumber = rand.nextInt(54) + 1;
            luckyArray[i] = luckyNumber;
        }
        //detect duplicates in the array
        for (int i = 0; i < luckyArray.length; i++) {
            if (i > 0 && luckyArray[i] == luckyArray[i - 1]) {
                System.out.print("WE HAVE FOUND A DUPLICATE!");
                /* while (luckyArray[i - 1] == luckyArray[i]) {
                    luckyArray[i] = rand.nextInt(54) + 1;
                }*/
            }
        }
        //sort the array before printing
        Arrays.sort(luckyArray);
        for (int t = 0; t < luckyArray.length; t++) {

            if (t != 5) {
                System.out.print(luckyArray[t] + "-");
            } else System.out.print(luckyArray[t]);
        }
    }
}

I expected to compare the current index [i] to that of its previous [i-1] and if/when they are equal, produce a new value for the current index.

Thanks in advance!

Jonathan Scialpi
  • 771
  • 2
  • 11
  • 32
  • That's not a good strategy. Read http://stackoverflow.com/questions/8115722/generating-unique-random-numbers-in-java – JB Nizet Sep 17 '15 at 22:24

5 Answers5

4

I would (personally) rely on the properties of a Set here to ensure that you're not retaining duplicates at all - you don't even have to check for dupes:

    Random rand = new Random();
    int numNumsToGenerate = 6;
    Set<Integer> luckySet = new HashSet<Integer>();

    //build set
    while (luckySet.size() < numNumsToGenerate) {
        luckySet.add(rand.nextInt(54) + 1);
    }

Once you've established your set, you can then sort them by creating a new List with the contents of your Set:

//sort the array before printing
List<Integer> luckyList = new ArrayList<Integer>(luckySet);
Collections.sort(luckyList);
Craig Otis
  • 31,257
  • 32
  • 136
  • 234
  • This is your best bet. I was just going to suggest this! – Evan LaHurd Sep 17 '15 at 22:26
  • Note that setting the capacity of the set to numNumsToGenerate is a bad idea. It's too small to contain the 6 numbers, and thus forces the set to rehash. You would need at least 8 to avoid a rehash. Doesn't matter much here, but in general using the expected size as the capacity is not something you should do. – JB Nizet Sep 17 '15 at 22:28
  • @JBNizet Can you elaborate? Why would it be too small to contain 6 numbers, when I give it an initial size of 6? – Craig Otis Sep 17 '15 at 22:29
  • @CraigOtis it's not a size. It's a capacity. As soon as the size reaches 75% of the capacity, the HashSet rehashes. It's a design mistake of HashSet, which should have taken an expected size rather than a capacity as argument. That's why Guava has newHashSetWithExpectedSize(int) – JB Nizet Sep 17 '15 at 22:31
  • @JBNizet Learned something new today! Thanks - would the edit suffice? (Specifying a `loadFactor` of `1.0` to ensure that the rehash won't happen until 7 or greater.) – Craig Otis Sep 17 '15 at 22:33
  • That's a bad idea. There's a good reason for the load factor not to be 1. With 1, you have a more chances of having hash conflicts, which reduce the performance of the Set (again, irrelevant for 6 entries, but important in general). The good way is to pass a good initial capacity: expectedSize + expectedSize / 3. This really only matters for large enough sizes. For small sizes, I don't bother setting a capacity, and just use the default. – JB Nizet Sep 17 '15 at 22:36
  • So generating 6 random non-repeating numbers is not possible with set @JBNizet ? – Jonathan Scialpi Sep 17 '15 at 22:58
  • 1
    @JonathanScialpi It's certainly possible (and a good approach), he was just commenting on the efficiency of providing a suggested capacity/load factor. – Craig Otis Sep 17 '15 at 23:09
  • 1
    @JonathanScialpi my advice is to ignore efficiency unless you are going to be running this code thousands of times a second. `HashSet` is a perfectly good collection for just about all practical situations. I recommend you avoid the tendency some coders have of making their code unnecessarily efficient at the cost of clarity. – sprinter Sep 17 '15 at 23:25
  • Tough for me not to accept this as the answer as it is the better non-Java8 solution, but the one I selected as the answer actually did get the code that I already had working. I upvoted it though, for sure http://meta.stackoverflow.com/questions/251843/accept-the-correct-answer-or-the-better-solution – Jonathan Scialpi Sep 17 '15 at 23:27
  • No problem @JonathanScialpi - happy coding! – Craig Otis Sep 17 '15 at 23:51
2

Move the 'sort' up, before you check for duplicates. This way you can be sure duplicates are always located next to each other and you only need to check neighbors.

Here the code that detects duplicates:

package luckynumber;

import java.util.Arrays;
import java.util.Random;

public class Main {

    public static void main(String[] args) {

        Random rand = new Random();
        int[] luckyArray = new int[6];

        // build array
        for (int i = 0; i < luckyArray.length; i++) {

            int luckyNumber = rand.nextInt(54) + 1;
            luckyArray[i] = luckyNumber;
        }
        // sort the array before detecting duplicates
        Arrays.sort(luckyArray);
        // detect duplicates in the array
        for (int i = 0; i < luckyArray.length; i++) {
            if (i > 0 && luckyArray[i] == luckyArray[i - 1]) {
                System.out.print("WE HAVE FOUND A DUPLICATE!");
                /* while (luckyArray[i - 1] == luckyArray[i]) { luckyArray[i] =
                 * rand.nextInt(54) + 1; } */
            }
        }
        for (int t = 0; t < luckyArray.length; t++) {

            if (t != 5) {
                System.out.print(luckyArray[t] + "-");
            } else
                System.out.print(luckyArray[t]);
        }
    }
}

When you now find a duplicate and change the array item make sure to re-sort and re-check the complete array. This remaining step is left as an exercise for the reader. ;-)

BTW: using Set will make the code much simpler.

Udo Borkowski
  • 311
  • 1
  • 9
2

Here is a very simple solution using Java 8 streams:

int[] luckyArray = new Random().ints(1, 55).distinct().limit(6).sorted().toArray();

This doesn't bother detecting duplicates manually - instead it uses distinct to remove them prior to choosing the first 6 and then sorting them.

sprinter
  • 27,148
  • 6
  • 47
  • 78
1

With Java 8:

List<Integer> luckys = 
    IntStream.rangeClosed(1, 54)
             .boxed()
             .collect(collectingAndThen(toList(), l -> {
                 Collections.shuffle(l);
                 return l;
             }))
             .stream()
             .limit(6)
             .collect(toList())
Jean Logeart
  • 52,687
  • 11
  • 83
  • 118
0

If sets are not an option you'll need to make a check that iterates through your array each time. Your current check only compares sequential number 1&2, 2&3 etc.

Alternately assign your lucky number to be checked to an int to be checked against your array before insertion.

accountget
  • 35
  • 6