Sieve of Eratosthenes with a Swing UI

I'm fairly new to Java. I want to divide my code into 3 classes. First class to contain my GUI, second to compute the algorithm and third class to write results in my file. I have the code working but I can't get to divide it in 3 separate classes. All i want to know is by example, how can i make parameter list that I calculate in Computer class to work in Writer class and so on. I wrote my full code , and then the attempts to divide it.

Full code

```
import javax.swing.*;
import java.awt.BorderLayout;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.io.*;
import java.util.ArrayList;
import java.util.List;

public class PrimeNumbers {

private JFrame frame;
private JTextField textField;

public static void main(String[] args) {

PrimeNumbers window = new PrimeNumbers();
window.frame.setVisible(true);
}

public PrimeNumbers() {
initialize();
}

public List Calculate() throws IOException {
String getTxt = textField.getText();
Integer x = Integer.parseInt(getTxt);
List list = new ArrayList<>();

boolean [] isComposite = new boolean [x + 1];
isComposite[1] = true;

for (int i = 2; i <= x; i++) {
if (!isComposite[i]) {

list.add(i);
int multiple = 2;
while (i * multiple <= x) {
isComposite [i * multiple] = true;
multiple++;
}
}
}
File file = new File("C:/Users/TudorV/Desktop/File.csv");

FileWriter fw = new FileWriter(file.getAbsoluteFile());
BufferedWriter bw = new BufferedWriter(fw);
bw.write(String.valueOf(list));
bw.close();
JOptionPane.showMessageDialog(frame, list);
return list;
}

private void initialize() {
frame = new JFrame();
frame.setBounds(100, 100, 450, 300);
frame.setDefaultCloseOperation(JFra

I started by moving the main method into a Main class.

public class Main {

    public static void main(String[] args) {
        Display display = new Display();
        display.start(new Calculator(), new Writer("C:/Users/TudorV/Desktop/File.csv"));
    }

}

This uses classes for Display, Calculator, and Writer. I renamed initialize to start because I changed the usage somewhat.

I moved the file name into this method so that it is easy to find and change. In a more serious program you might make it an argument (args) or add it to the form in Display.

Display is mostly similar to your original PrimeNumbers class.

public class Display {

    private JFrame frame = new JFrame();
    private JTextField textField = new JTextField();

    public void start(Calculator calculator, Writer writer) {
        frame.setBounds(100, 100, 450, 300);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        frame.getContentPane().add(textField, BorderLayout.NORTH);
        textField.setColumns(10);

        JButton btnGetPrimeNumbers = new JButton("Get prime numbers");
        btnGetPrimeNumbers.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                String getTxt = textField.getText();
                int x = Integer.parseInt(getTxt);
                List list = calculator.calculate(x);

                writer.write(list);

                JOptionPane.showMessageDialog(frame, list);
            }
        });

        frame.getContentPane().add(btnGetPrimeNumbers, BorderLayout.SOUTH);
        frame.setVisible(true);
    }

}

However, I moved the calculations and file writing out of this class. I pass in objects to handle those responsibilities. They get called by the ActionListener on the button as you did originally.

Skipping down to Writer

public class Writer {

    String fileName;

    public Writer(String fileName) {
        this.fileName = fileName;
    }

    public void write(List list) {
        File file = new File(fileName);
        try (BufferedWriter bw = new BufferedWriter(new FileWriter(file.getAbsoluteFile()))) {
            bw.write(String.valueOf(list));
        }
    }

}

I added a new field (fileName) and set it in the constructor.

If you're just printing the stack trace, there isn't much point in catching the exception. That's the default behavior anyway.

I changed the code to use try-with-resources. This ensures that the BufferedWriter gets closed even if there is an exception.

Going back to Calculator

public class Calculator {

    public List calculate(int ceiling) {
        boolean [] composites = new boolean[ceiling + 1];

        List primes = new ArrayList<>();
        for (int i = 2; i <= 3 && i <= ceiling; i++) {
            primes.add(i);
        }

        int increment = 4;
        for (int i = 5; i < composites.length; i += increment) {
            if (!composites[i]) {
                primes.add(i);
                for (int j = i * i; j < composites.length; j += i) {
                    composites[j] = true;
                }
            }

            increment = 6 - increment;
        }

        return primes;
    }

}

As you may have noticed, I made a few changes.

I made calculate lower case, as that is the standard for single word Java method naming (a special instance of camelCase).

I changed x to ceiling as a more descriptive name. And list to primes for the same reason. I changed isComposite to composites because variables should have noun names.

Rather than check every number, I skip all the even numbers and odd numbers divisible by three. It's easy to skip evens by adding 2 instead of just incrementing by 1. To skip the numbers divisible by three, we alternate incrementing by 2 and 4. This produces 5, 7, skip 9 (because we increment 7 by 4), 11, 13, skip 15, 17, 19, skip 21...

That requires special logic to handle 2 and 3.

I changed the actual sieving code from a while loop to a for loop.

You start with a multiple of 2 and multiply each iteration. I started with the square. Realize that you already marked the smaller multiples as composites. No need to do it again. Similarly, if we increment by the number rather than 1, we can make j the actual number that needs marked. That saves a multiplication each iteration.

Note: I didn't look much at your Attempts code. It didn't follow the approach that I would take. In particular, I saw no reason to use extends to relate those classes. When you use inheritance, it should generally be because one class is a special instance of another. But even then, you often should prefer composition (including fields of the relevant type) to inheritance.

StackExchange Code Review Q#133502, answer score: 3