Ben E.
Ben E.

Reputation: 1

Code not functioning correctly identifying prime numbers in an array

This code is supposed to firstly, run a code that gets the factors of a number, and secondly run a code to check numbers in an array and remove any prime numbers.

import java.util.*;
public class ArrayListFunHouse {
Scanner keyboard = new Scanner(System.in);
private static int input;
private int keepOnlyCompositeNumbersSize;
private static ArrayList<Integer> onlyCompositeNumbers = new ArrayList<Integer>();
private static ArrayList<Integer> getListOfFactors = new ArrayList<Integer>();
private static ArrayList<Integer> keepOnlyCompositeNumbers = new ArrayList<Integer>();
private static ArrayList<Integer> getListOfFactorsComposite = new ArrayList<Integer>();
public void setInput() {
    System.out.print("Please input the input you want to check the factors of :: ");
    input = keyboard.nextInt();
    System.out.print("Please enter the size of the array you wish to find the composite numbers in :: ");
    keepOnlyCompositeNumbersSize = keyboard.nextInt();
    for(int spot = 0; spot < keepOnlyCompositeNumbersSize; spot++) {
        keepOnlyCompositeNumbers.add(keyboard.nextInt());
    }
}
public static ArrayList<Integer> getListOfFactors() {
        for(int i = 1; i <= input; i++) {
        if(input % i == 0 && i!= 1 && i != input) {
            getListOfFactors.add(i);
        }
    }
    return getListOfFactors;
}
public static ArrayList<Integer> getListOfFactorsCompositeList(int number) {
    for(int i = 1; i <= number; i++) {
        if(number % i == 0 && i!= 1 && i != number) {
            getListOfFactorsComposite.add(i);
        }
    }
    return getListOfFactorsComposite;
}
public static ArrayList<Integer> keepOnlyCompositeNumbers() {
    for(int i = 0; i < keepOnlyCompositeNumbers.size();i++) {
        if(getListOfFactorsCompositeList(keepOnlyCompositeNumbers.get(i)).isEmpty() == false) {
            onlyCompositeNumbers.add(keepOnlyCompositeNumbers.get(i));
        }
    }
    return onlyCompositeNumbers;
}
public static void print() {
    System.out.println(getListOfFactors);
    System.out.println("Original list");
    System.out.println(keepOnlyCompositeNumbers);
    System.out.println("Composite List");
    System.out.println(onlyCompositeNumbers);
    }
}

This is the runner:

public class ArrayListFunHouseRunner {

public static void main(String[] args) {
    // TODO Auto-generated method stub
    ArrayListFunHouse A = new ArrayListFunHouse();
    A.setInput();
    ArrayListFunHouse.getListOfFactors();
    ArrayListFunHouse.keepOnlyCompositeNumbers();
    ArrayListFunHouse.print();
}

}

Input:

Array Size: 15
Numbers: 2 6 8 9 10 12 13 15 17 24 55 66 78 77 79

Expected output is

[6, 8, 9, 10, 12, 15, 24, 55, 66, 78, 77]

Actual output is

[6, 8, 9, 10, 12, 13, 15, 17, 24, 55, 66, 78, 77, 79]

Upvotes: 0

Views: 139

Answers (2)

Frakcool
Frakcool

Reputation: 11153

I think you don't understand how to pass parameters and how to use objects.

The following code has a method that knows if a number is prime and if that method returns true, then we don't add those numbers to the new list.

It also makes use of an inner class, and I think it's clearer to understand what's happening than in your code.

Note that I removed the System.out.println(""); calls so your input is like this one:

Input:

2 6 8 9 10 12 13 15 17 24 55 66 78 77 79

Output:

[6, 8, 9, 10, 12, 15, 24, 55, 66, 78, 77]

As you can see, the output is correct and the code is shorter and clearer

Code:

import java.util.ArrayList;
import java.util.Scanner;

public class FunHouseRunner {
    public static void main(String[] args) {
        FunHouse funHouse = new FunHouse();
        ArrayList<Integer> userInputList = funHouse.getUserInput(15);
        ArrayList<Integer> listWithoutPrimeNumbers = funHouse.getListWithoutPrimeNumbers(userInputList);
        System.out.println(listWithoutPrimeNumbers);
    }
}

class FunHouse {

    private Scanner sc = new Scanner(System.in);

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

    public ArrayList <Integer> getUserInput(int size) {
        ArrayList <Integer> userInputList = new ArrayList<Integer>();

        for (int i = 0; i < size; i++) {
            userInputList.add(sc.nextInt());
        }

        return userInputList;
    }

    public ArrayList <Integer> getListWithoutPrimeNumbers(ArrayList <Integer> userInputList) {
        ArrayList <Integer> listWithoutPrimeNumbers = new ArrayList<Integer>();

        for (int n : userInputList) {
            if (!isPrime(n)) {
                listWithoutPrimeNumbers.add(n);
            }
        }

        return listWithoutPrimeNumbers;
    }

    public void printList(ArrayList<Integer> listWithoutPrimeNumbers) {
        System.out.println(listWithoutPrimeNumbers);
    }
}

Upvotes: 0

VHS
VHS

Reputation: 10184

Change

public static ArrayList<Integer> getListOfFactorsCompositeList(int number) {
    for(int i = 1; i <= number; i++) {
        if(number % i == 0 && i!= 1 && i != number) {
            getListOfFactorsComposite.add(i);
        }
    }
    return getListOfFactorsComposite;
}

to

public static ArrayList<Integer> getListOfFactorsCompositeList(int number) {
    getListOfFactorsComposite = new ArrayList<Integer>();
    for(int i = 1; i <= number; i++) {
        if(number % i == 0 && i!= 1 && i != number) {
            getListOfFactorsComposite.add(i);
        }
    }
    return getListOfFactorsComposite;
}

Basically, you just need to add one line getListOfFactorsComposite = new ArrayList<Integer>(); in the beginning of the method getListOfFactorsCompositeList

Reason

You have declared getListOfFactorsComposite as an instance field of List type. So between the method calls, it is retaining the elements added in the prior method calls. Addition of this line is required to reset the List prior to new use.

Upvotes: 2

Related Questions