mtldev1642
mtldev1642

Reputation: 59

Read data into an arraylist

I made a very simple program with a GUI where the user inputs the file name and the program itself calculates the mean of a file containing numbers only. I did this by creating a function that reads each line into an ArrayList and then compute the mean but the scanner I've created ran into some issues. Edit: There's only one number per line

import java.io.File;
import java.util.Scanner;
import java.util.ArrayList;
import javax.swing.JOptionPane;

public class MainClass {

    
    public static double mean_file(String file_name) {
        /*
         * Calculates the average of numbers from a text file
         */
        File data = new File(file_name);
        double sum = 0;
        ArrayList <Integer> list_of_numbers = new ArrayList<Integer>();
        Scanner read = new Scanner(data);
        
        while (read.hasNextLine()) {
            try {
                int nb = Integer.parseInt(read.nextLine());
                list_of_numbers.add(nb);
            } catch (NumberFormatException n) {
                JOptionPane.showOptionDialog(null, "File content error", "Files must only contain one number per line!", 0, 1, null, null, "OK");
            }
        }
        for (int c=0; c<list_of_numbers.size(); c++) {
            sum += list_of_numbers.get(c);
        }
        double mean = sum / list_of_numbers.size();
        
        return mean;
    }

}

Here's the code for the GUI

import java.awt.EventQueue;

import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JTextField;
import javax.swing.JButton;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import javax.swing.JTextArea;

public class TextFileMean {

    private JFrame frame;
    private JLabel FileNameLabel;
    private JTextField FileNameEntry;
    private JButton OpenFileBtn;
    private JTextArea Result;

    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    TextFileMean window = new TextFileMean();
                    window.frame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
    }

    /**
     * Create the application.
     */
    public TextFileMean() {
        initialize();
    }

    /**
     * Initialize the contents of the frame.
     */
    private void initialize() {
        frame = new JFrame();
        frame.setBounds(100, 100, 450, 300);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.getContentPane().setLayout(null);
        
        FileNameLabel = new JLabel("File name");
        FileNameLabel.setBounds(65, 92, 71, 16);
        frame.getContentPane().add(FileNameLabel);
        
        FileNameEntry = new JTextField();
        FileNameEntry.setBounds(148, 87, 230, 26);
        frame.getContentPane().add(FileNameEntry);
        FileNameEntry.setColumns(10);
        
        OpenFileBtn = new JButton("Open file");
        OpenFileBtn.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent e) {
                double mean = MainClass.mean_file(FileNameEntry.getText());
                Result.setText(Double.toString(mean));
            }
        });
        OpenFileBtn.setBounds(164, 125, 117, 29);
        frame.getContentPane().add(OpenFileBtn);
        
        Result = new JTextArea();
        Result.setBounds(165, 191, 100, 30);
        frame.getContentPane().add(Result);
    }

}

I expect the function created to read each line and add it to the ArrayList but the scanner itself raised a FileNotFoundException and a resource leak error. I tried using the file itself as the parameter and the file name (a string from which we'll open the file using a constructor) and none of them worked.

Upvotes: 5

Views: 138

Answers (2)

Basil Bourque
Basil Bourque

Reputation: 339837

The Answer by K.Sx0000 is correct, and smart. Read and up-vote that Answer before this Answer.

Beyond that, we can revamp your code to (a) be simpler and shorter, and (b) respect separation of concerns. Plus, some tips.

As a beginner, try to avoid using static in your code. Such code is not object-oriented. While occasionally helpful, static tends to be overused and abused by those learning Java.

Use smart names that describe the purpose of a class, method, variable, etc. So rather than MainClass, IntegersFileReader.

Regarding separation of concerns, your code is mixing three concerns in the mean_file method: File reading & parsing, math, and user-interface. Instead, break those up. One class for reading and parsing, another for the math. The user-interface part should be covered by your other existing user-interface code.

Your code involves Scanner but makes no good use of that class. The class is for sophisticated parsing. But you ignore the features of Scanner, and parse the numbers yourself in your own code. So drop the Scanner.

File reading & parsing

Our first task is reading the file’s lines, and parsing them as integers.

For handling files in modern Java, study NIO.2. Learn about the utility methods in Files & Paths classes (plural names, each).

Generally avoid using underscores in names in Java. By convention, use camelCase. So, fileName rather than file_name. Instead of list_of_numbers, numbers — or better yet, integers to be more specific. The exception is for constants which should be in all uppercase, ex: HOME_FOLDER.

Putting that together, we have a integers-file-reading class that looks like this:

package work.basil.example.swing.integersfile;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;
import java.util.SequencedCollection;

class IntegersFileReader
{
    SequencedCollection < Integer > read ( final String fileName ) throws IOException, NumberFormatException
    {
        Objects.requireNonNull ( fileName );
        if ( fileName.isBlank ( ) ) { throw new IllegalArgumentException ( "Received blank text instead of file name." ); }

        // Find file.
        final String HOME_FOLDER = System.getProperty ( "user.home" );
        Path path = Paths.get ( HOME_FOLDER , fileName );
        boolean exists = Files.exists ( path );
        if ( ! exists ) { throw new IllegalArgumentException ( "No such file name found in home folder: " + fileName ); }

        // Read lines of file.
        SequencedCollection < String > lines = Files.readAllLines ( path );

        // Parse lines as integer numbers.
        SequencedCollection < Integer > integers = lines.stream ( ).map ( Integer :: valueOf ).toList ( ); // Throws NumberFormatException.

        // Result.
        return integers;
    }

}

Notice in this code above that we assume the desired file is to be found in the user’s home folder. In your situation, the question of what folder contains the desired file may be a part of your problems. See Comment by Bohemian. Notice in this code how we explicitly check for the existence of the file by calling Files.exists.

Testing

One very nice benefit of separating concerns is the ability to test each concern separately. Let's write a bit of code to try the class above.

package work.basil.example.swing.integersfile;

import java.io.IOException;
import java.util.SequencedCollection;

public class Testing
{
    public static void main ( String[] args )
    {
        Testing testing = new Testing ( );
        testing.testFileReader ( );
    }

    private void testFileReader ( )
    {
        String fileName = "integers.text";
        IntegersFileReader reader = new IntegersFileReader ( );
        try
        {
            SequencedCollection < Integer > integers = reader.read ( fileName );
            integers.forEach ( System.out :: println );
        }
        catch ( IOException e )
        {
            throw new RuntimeException ( e );  // In production code, replace with your own sensible handling.
        }
    }
}

When run, we find success.

7
42
99

Calculating mean

Put your math code in a MathUtils class. Here a static method makes sense because we are nearly certain this class will never carry state.

public class MathUtils
{
    public static double mean ( final SequencedCollection < Integer > integers )
    {
        if ( integers.isEmpty ( ) ) return 0;

        List < Integer > list = List.copyOf ( integers );
        int sum = 0;
        for ( int index = 0 ; index < list.size ( ) ; index++ )
        {
            sum += list.get ( index );
        }
        System.out.println ( "sum = " + sum );
        double mean = sum / list.size ( );

        return mean;
    }
}

Let's write a test. We add a testMean method to our Testing class.

    private void testMean ( )
    {
        double mean = MathUtils.mean ( List.of ( 7 , 42 , 99 ) );
        System.out.println ( "mean = " + mean );
    }

Integer math versus Fractional math

When run:

mean = 49.0

Ooooh, not good. The average of those three numbers is not a whole number. So we have detected a bug in your logic. Your line double mean = sum / list_of_numbers.size(); is troubled in that both your numerator and your denominator in that division are int values. So you are telling Java to perform integer math rather than fractional math. Fix this by casting either the numerator or the denominator to be a double.

       double mean = ( double ) sum / list.size ( );  // Must cast either numerator or denominator to perform double math rather than integer math.

When we run our test again, we get a fractional number as expected.

mean = 49.333333333333336

Integer overflow

Are we done with the math? No. The logic in your original code, and here in this modified version, has another source of bugs.

Declaring sum as int means that with a sufficient number of integers of sufficiently large value, we could exceed the maximum int number of Integer.MAX_VALUE, = 231-1, ≅ 2 billion. This leads to integer overflow, with erroneous results.

To detect integer overflow, replace your use of the + operator with a call to Math.addExact. In the event of overflow, that method throws ArithmeticException.

To avoid integer overflow, declare your sum as long rather than int, a 64-bit number rather than 32-bit.

Or just use double, as that’s our goal in your case anyways. When defining sum as double, our need for the cast ( double ) evaporates.

And we can replace the for statement with an enhanced for.

    public static double mean ( final SequencedCollection < Integer > integers )
    {
        if ( integers.isEmpty ( ) ) return 0;

        List < Integer > list = List.copyOf ( integers );
        double sum = 0;
        for ( Integer integer : list )
        {
            sum += integer;  // The `Integer` object is auto-boxed to an `int`, which is automatically converted to a `double`. 
        }
        System.out.println ( "sum = " + sum );
        double mean = sum / list.size ( );

        return mean;
    }

Again, let me point out how nice it is to be able to test, debug, fix, and rewrite parts of your program rather than all the code at once.

Stream instead of loop

Indeed, we could shorten our implementation of our mean method with a drastic rewrite. We can replace the enhanced-for loop with a Stream. (See this post).

Conceptually, the same work is being done: (a) move through the elements of the SequencedCollection of integer numbers, (b) convert the integers to double values, (3) calculate the mean. If the list of integers were empty, we return a result of zero.

    public static double mean ( final SequencedCollection < Integer > integers )
    {
        return
                integers
                        .stream ( )
                        .mapToDouble ( Integer :: doubleValue )
                        .average ( )
                        .orElse ( 0.0d );
    }

Now we can simply run our same test to make sure this new code works correctly.

mean = 49.333333333333336

Adopting this code

You can integrate this code into your app by having the user-interface make calls to these two classes in a fashion similar to the testing code we wrote.

Notice that our read method above declares that it throws IOException, NumberFormatException. These are the exceptions that your user-interface should expect to catch as both are due to common errors that are entirely possible: Faulty file name or location, and incorrect data within that file.

Circling back to your original code, and the issues identified by the Answer by K.Sx0000, this new code here avoids all those problems:

  1. We throw IOException for your UI to deal with problems locating or opening the file.
  2. We have no Scanner resource opened, so no need to close. The Files.readAllLines call handles opening and closing the file.
  3. Your UI code can catch any exceptions for both file-reading and file-parsing problems.

Upvotes: 4

K.Sx0000
K.Sx0000

Reputation: 122

You are seeing these errors because:

  1. Your mean_file function in MainClass needs to report any FileNotFoundException that can be caused by the line Scanner read = new Scanner(data);. You can fix this by adding a throws declaration to the method declaration like this:

    public static double mean_file(String file_name) throws FileNotFoundException

  2. You are forgetting to close the scanner which is causing the resource leak error. You can fix this by adding read.close(); right after the while loop in MainClass.

  3. In your TextFileMean class, in the mouseClicked() method, you need surround your call to the mean_file() method with a try/catch block to handle any FileNotFoundException. You can fix that like this:

     public void mouseClicked(MouseEvent e) {
        double mean = 0;
        try {
            mean = MainClass.mean_file(FileNameEntry.getText());
        } catch (FileNotFoundException e1) {
            e1.printStackTrace();
        }
        Result.setText(Double.toString(mean));
     }
    

Note that you need to initialize the mean variable, in this example I've initialized it to 0.

After all these changes, I was successfully able to run your code.

Upvotes: 2

Related Questions