CODEWITHSUNDEEP
java
Danilo
Danilo

Reputation: 23

If statement executed even if the condition is false

I have a method that generate random number but i want some of them to be trashed. This is the code:

    public static int getRandomX(int max, int[] exclude){
        Random randomGenerator = new Random();
        Integer[] toExclude = Arrays.stream( exclude ).boxed().toArray(Integer[]::new);
        Integer random = Integer.valueOf(randomGenerator.nextInt(max));
        do{
            System.out.println(!Arrays.asList(toExclude).contains(random));
            if(!(Arrays.asList(toExclude).contains(random))){
                return random;
            } else{
                random ++;
            }
        }while(!Arrays.asList(toExclude).contains(random));
        return random;
    }

Even if the System.out.println(!Arrays.asList(toExclude).contains(random)); print false the if get executed and i get a wrong random number

Upvotes: 0

Views: 89

Answers (2)

Eritrean
Eritrean

Reputation: 16508

I'd rather work with sets than lists. The advantage is you can add an element to a set and depending on whether the element already exists or not set.add returns true or false, making your while loop easier and more readable:

public static int getRandomX(int max, int[] exclude){
    Set<Integer> set = Arrays.stream(exclude).boxed().collect(Collectors.toSet());
    Random randomGenerator = new Random(max);
    Integer random = randomGenerator.nextInt(max);
    while(!set.add(random)){
        random = randomGenerator.nextInt(max);
    }
    return random;
}

But be careful! Both this approach and that of @Nicholas K ends in an endless loop if exclude contains all numbers between 0 and max-1. To prevent this, add a validation of the arguments as given below. There is also another advantage of sets over lists since you don't have to filter duplicates first. 

public static int getRandomX(int max, int[] exclude){
    Set<Integer> set = Arrays.stream(exclude).boxed().collect(Collectors.toSet());
    if(set.size() == max){
        throw new IllegalArgumentException("All numbers in range [0 - " +(max-1)+"] excluded");
        //or return -1;
    }
    Random randomGenerator = new Random(max);
    Integer random = randomGenerator.nextInt(max);
    while(!set.add(random)){
        random = randomGenerator.nextInt(max);
    }
    return random;
}

Upvotes: 1

Nicholas K
Nicholas K

Reputation: 15443

There is an incorrect logic in the while loop. You need to execute the loop as long as there is a number that needs to be excluded and not the other way around.

Simply your code to:

public static int getRandomX(int max, int[] exclude) {
    Random randomGenerator = new Random();
    Integer[] toExclude = Arrays.stream(exclude).boxed().toArray(Integer[]::new);
    Integer random;
    do {
        random = Integer.valueOf(randomGenerator.nextInt(max));
    } while (Arrays.asList(toExclude).contains(random));
    return random;
}

Upvotes: 2

Related Questions