# Checking an array for duplicate values

This function checks an array for duplicated values. If it finds a duplicate value, it increments the variable name repeatedTime and then if repeatedTime is greater than 1 it calls a randomize function which produce different values in the array that was passed to the function. It repeats these steps until a duplicated value free array is produced and then notDuplicate becomes true and the function ends.

Are there any improvements that can be made to the function? I am thinking about making it into a recursive function. Is that feasible?

private void CheckForDuplicates(int[] selectedWordIds, List<Integer> answersTempId) {
boolean notDuplicate = false;

while(notDuplicate != true){
int repeatedTime = 0;
for(int x = 0; x < selectedWordIds.length ; x++)
{
for(int y =0; x < selectedWordIds.length;x++)
{
if(selectedWordIds[x] == selectedWordIds[y])
{
repeatedTime++;
}
}
}
if(repeatedTime > 1)
{
notDuplicate = false;
}
else{
notDuplicate    = true;
}
}
}

• Why do you need answersTempId? Perhaps it plays a role in your Randomization method however that is not clear. Commented Dec 29, 2014 at 20:03

I suggest you use a Set when checking for duplicates:

Set<Integer> set = new HashSet<>();

for(int i : selectedWordIds) {
repeatedTime++;
}
}


This is much simpler that what you have.

Here:

 if(repeatedTime > 1)
{
notDuplicate = false;
}


The assignment notDuplicate = false is not necessary because notDuplicate is already false.

About recursion: It is possible. With it, here is the final code:

private void CheckForDuplicates(int[] selectedWordIds,

int repeatedTime = 0;

Set<Integer> set = new HashSet<>();

for(int i : selectedWordIds) {
repeatedTime++;
}
}
if (repeatedTime > 1) {
}

}

• Recursion is possible but not called for. All you're doing is keeping memory allocated in the heap for longer than necessary. Commented Dec 30, 2014 at 2:54
• While I like recursion and use it frequently myself in Java, we need to keep in mind that Java is (Oracle, are you listening?!!!) not performing tail recursion optimization, i.e. every single recursive step of a recursion really allocates a new stack frame in Java. Commented Dec 31, 2014 at 3:13
• I'd expect that at the very first iteration (when both x and y are 0) the selectedWordIds[x] == selectedWordIds[y] expression would be true no matter what.

• Single Responsibility Principle dictates that CheckForDuplicates shall check for duplicates, no more no less. Randomization is not in its scope.

Leveraging both existing answers (@MannyMeng, @vnp).

Some notes:
1. It's bad form for randomizeFunction() to mutate the array that's passed in. Better would be if it returned a new array instead, but that's out of scope for what you asked to be reviewed.

private boolean hasDuplicates(final int[] selectedWordIds) {

final Set<Integer> set = new HashSet<>();

for (final int i : selectedWordIds) {
return true;
}
}
return false;
}

private int[] removeDuplicates(
final int[] selectedWordIds,