Prime Directive

Continuing the discussion from Prime Directive:

Hi,

I just wanted to show you the new, corrected code which seems to work. I followed your advice and created two separate functions for the odd and for the even numbers (as I did to begin with but kept running into problems at first).
Actually, all I did was to replace the ! inside the brackets in the evenNumbers method, and surprisingly, it solved my problem. I thought exclamation marks needed to be put outside not inside the brackets but I was obviously mistaken.

import java.util.ArrayList;

class PrimeDirective {

public boolean isPrime(int number){
if (number == 2){
return true;
}
else if (number < 2){
return false;
}

for (int i = 2; i < number; i++){
if (number % i == 0) {
return false;
}
}
return true;
}

public ArrayList onlyPrimes(int numbers){
ArrayList primes = new ArrayList();
for (int number : numbers){
if (isPrime(number)){
primes.add(number);
}
}
return primes;
}

public boolean isOdd (int number){
if (number % 2 != 0) {
return true;
}
else {
return false;
}
}

public ArrayList oddNumbers (int numbers){
ArrayList onlyOdds = new ArrayList();
for (int number : numbers){
if (isOdd(number)){
onlyOdds.add(number);
}
}
return onlyOdds;
}

public ArrayList evenNumbers (int numbers){
ArrayList onlyEven = new ArrayList();
for (int number : numbers){
if (!isOdd(number)){
onlyEven.add(number);
}
}
return onlyEven;
}

public static void main(String args) {

PrimeDirective pd = new PrimeDirective();
int[] numbers = {6, 29, 33, 100, 11, 101, 43, 89};
System.out.println(pd.evenNumbers(numbers));  

}
}

Pat

You really should put your code in a code block. It took me a good 10-15 minutes just to format the code in a file so I could read it and get an idea of what’s going wrong.

First I’ll say, this isn’t at all the way I would do it. I don’t think there is necessarily a wrong answer here but since you are doing lessons and I don’t want to take you too far out of scope if you don’t want to be, I will give you 2 answers. The direct fix to your current situation and how I would do it. Feel free to ignore one in favor of the other as you see fit.

The biggest problems I see are your collections. Primarily the arrays. You are passing an int into the methods instead of an array and then using a for loop to iterate through it.

You have:

    public ArrayList onlyPrimes(int numbers) {
        ArrayList primes = new ArrayList();
        for (int number : numbers) {
            if (isPrime(number)) {
                primes.add(number);
            }
        }
        return primes;
    }

It should be:

    public ArrayList onlyPrimes(int[] numbers) {
        ArrayList primes = new ArrayList();
        for (int number : numbers) {
            if (isPrime(number)) {
                primes.add(number);
            }
        }
        return primes;
    }

Notice the addition of [ and ] to int. This is how you tell java it’s an array of that variable type and not just the variable type itself. You do this in 4 places: onlyPrimes(), oddNumbers(), evenNumbers(), and main(). The main() one is actually the worst one. It means your code won’t run at all because you aren’t passing a String array which is required by Java to run :wink: fix those problems and the code will run.

Now the bit where I deep dive and tell you things you may not have learned yet. If you want to take the red pill and see it, by all means. If you would rather not and stick to the lessons, I understand. Just click the blurred spoiler to see the more complicated version.

The first thing to note is you are passing a raw ArrayList around. This is fine for the moment but it’s probably a bad habit and I disagree with it being taught this way blah blah blah. It’s not a big deal but you should use carrots to tell Java what variable the ArrayList contains like this: ArrayList<Integer>

Again, not a big deal minor nit pick on my part for where you’re at. Just know that when you do this with primitives (int, boolean, float, etc) you have to use the whole class name, not just the keyword like when you assign a variable. It’s not intuitive at all but it’s something to know.

The better way to do this whole thing though, is to pass the int array into the class as a variable when you make the object in the main() method. That’s the line where you are using the ‘new’ keyword. Then your methods can just work on that variable. It looks something like this (comments added for clarity):

import java.util.ArrayList;

class PrimeDirective {

    // This is the class variable. It is outside the methods but inside the class, so all methods will have access to it at the same time.
    int[] whiteboard;

    // This is the constructor. In plain english, it's the method that runs when you use "new PrimeDirective()" and we can use is to assign the class variable mentioned above.
    public PrimeDirective(int[] input){
        // This is where we take the input and pass it to our class variable
        this.whiteboard = input;
    }

    // NOTE: I'm going to move our "helper methods" around just to focus on the ones doing the actual work. Noting it here so you know I've done that.

    public ArrayList<Integer> onlyPrimes() {
        // I changed this to reflect my little nit pick on variable type assignments to ArrayList ;)
        ArrayList<Integer> primes = new ArrayList<Integer>();

        // The biggest change here is the use of this.whiteboard instead of numbers as an array from the method call. In plain english: Iterate through all numbers in the class variable whiteboard.
        for (int number : this.whiteboard) {
            if (isPrime(number)) {
                primes.add(number);
            }
        }
        return primes;
    }

    public ArrayList<Integer> oddNumbers() {
        // Same here as onlyPrimes()
        ArrayList<Integer> onlyOdds = new ArrayList<Integer>();

        // Same here as onlyPrimes()
        for (int number : this.whiteboard) {
            if (isOdd(number)) {
                onlyOdds.add(number);
            }
        }
        return onlyOdds;
    }

    public ArrayList<Integer> evenNumbers() {
        // Same here as onlyPrimes()
        ArrayList<Integer> onlyEven = new ArrayList<Integer>();

        // Same here as onlyPrimes()
        for (int number : this.whiteboard) {
            if (!isOdd(number)) {
                onlyEven.add(number);
            }
        }
        return onlyEven;
    }

    // I am adding this to answer the question "Why is this better". With this method we can reassign the class variable any time we want. I'll show that off in the main method.
    public void setArray(int[] input) {
        this.whiteboard = input;
    }

    // These are your 2 helper methods. They are fine as is and I would probably do the same thing.
    public boolean isOdd(int number) {
        if (number % 2 != 0) {
            return true;
        } else {
            return false;
        }
    }

    public boolean isPrime(int number) {
        if (number == 2) {
            return true;
        } else if (number < 2) {
            return false;
        }

        for (int i = 2; i < number; i++) {
            if (number % i == 0) {
                return false;
            }
        }
        return true;
    }


    // The main run method. Normally would be in a class dedicated to starting the program but it's ok that it's here. I will draw your eye to the comments below though.
    public static void main(String[] args) {
        // We want to create the array before the object. Why? So we can pass it into the constructor of course :)
        int[] numbers = { 6, 29, 33, 100, 11, 101, 43, 89 };

        // Note how I am passing the array into this method call (referred to as a constructor) here. This is where the magic happens from the top lines of code.
        PrimeDirective pd = new PrimeDirective(numbers);
        
        // This is the power of what this lets us do. Now you can simply call onlyPrimes() without any sort of method call. It just returns the result from the class variable.
        System.out.println(pd.onlyPrimes());
        
        // or evenNumbers()
        System.out.println(pd.evenNumbers());
        // or oddNumbers()
        System.out.println(pd.oddNumbers());
        
        // All done with the same object we created 4 lines of code ago


        // Here is me showing off what this method does.
        int[] numbersAgain = { 8, 10, 55, 87, 42, 50 };

        // We don't even have to make a new object now, just use the same one
        pd.setArray(numbersAgain);

        System.out.println(pd.evenNumbers());

    }

}

I could have made more notes on this subject. There is so much more, but I wanted to keep it fairly simple to just demonstrate classes. Especially since this is a situation where they come in handy.

1 Like