7
\$\begingroup\$

My code picks a value from a too large csv file (~300MB.) at a given index i and j (assuming the csv as 2D array because it contains just data without headers), it works but it is slow. Is there other method to pick a value from csv file faster?

import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.FileInputStream;
import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.ArrayList;

public class Main
{

    public static String r1(String input,int i, int j,int bufferSize){

        String[][] result = {};
        String row ="";

        try {
            BufferedReader r1 = new BufferedReader(new InputStreamReader(new FileInputStream(new File(input)), "UTF-8"), bufferSize);

            List<String[]> lines = new ArrayList<String[]>();

            while((row=r1.readLine())!=null){
                lines.add(row.split(","));
            }

            String[][] csv = new String[lines.size()][0];
            lines.toArray(csv);
            result = csv;
            r1.close();
        }

        catch(IOException e){
            e.printStackTrace();
        }
        return result[i][j];
    }

    public static void main(String[] args){
        int bufferSize = 10 * 1024;   //10 KB.
        String filename = "matrix.csv"; 
        int i = 744, j = 7000;
        String pickedValue = r1(filename,i,j,bufferSize);
        System.out.println(pickedValue);
    }


}
\$\endgroup\$
8
\$\begingroup\$

Spaces

Your use of spaces could stand improvement. You should have one space after every comma in argument list, and around binary operators. It is also customary to have a space between a closing parenthesis and an opening brace.

Names

Use descriptive names!

Is r1 a method name, or a BufferedReader? Why are these named the same, anyway?

What is input? It is a String, that much is clear, but what is it used for? filename would be so much more descriptive than input.

Try-with-resources

If an IOException happens, is FileInputStream closed, or does it bypass the r1.close() statement?

Java 7 introduced the "try-with-resources" construct, which ensures resources are properly closed, even in the event of exceptions, and even if exceptions happen during the exception handling.

try (FileInputStream fis = new FileInputStream(new File(input)),
         InputStreamReader isr = new InputStreamReader(fir, "UTF-8"),
         BufferedReader br = new BufferedReader(isr)) {

    // use br here - see below


    // br, isr, and fis are automatically closed here, at the end of the try block.
} catch (IOException e) {
    e.printStackTrace();
}

Efficiency

You are doing a lot of unnecessary work, wasting time, to extract a single value from the file.

  • If you want the value from the first line, you still read in all of the lines
  • You split every line into columns, even when you don't need a value from that line.
  • You store all of the lines of data, split into columns, as an ArrayList
  • You store a second copy of of all of the lines of data, as a String[][]

In place of the // use br here - see below line, you could do the following:

  • Read in lines without storing them, until you reach the i-th row.
  • Split the i-th row into columns
  • Return the j-th value

Ie)

for(int row = 0;  row < i;  row++) {
    br.readLine();                    // Note: value is not saved
}

String line = br.readLine();
String[] columns = line.split(",");
return columns[j];                    // Resources are automatically closed!

Remove the original declaration of result, row, and lines.


What about bufferSize, should I never use it?

It was an oversight on my part when writing the try-with-resources replacement, but in general, no. The JVM will pick a buffer size that is optimal for the OS.

If you did want to specify a buffer size, 10KiB seems too arbitrary. It should be related to (a multiple of) the size of the data being read, in this case, the length of the lines in the csv file. The larger the value, the lower the odds that readline() will have to do string concatenation because the part of the line is in one buffer & part of the line is in another. However, the larger the value, the higher the odds that too much data will be read (you want to stop at exactly the right line, remember?), memory pressure will increase causing other inefficiencies. If you do try tweaking the value, profile, profile, profile (... and then go back to the default).

\$\endgroup\$
7
  • \$\begingroup\$ I appreciate your work, thank you. I fixed the code. \$\endgroup\$ – Khaled Nov 5 '19 at 12:01
  • \$\begingroup\$ But what about BufferSize, should I never use it? \$\endgroup\$ – Khaled Nov 5 '19 at 12:04
  • 1
    \$\begingroup\$ @Khaled Unless you have some reason to override the default buffer size, don't. \$\endgroup\$ – JollyJoker Nov 5 '19 at 12:50
  • \$\begingroup\$ Speed of read and writing is greatly influenced by the size of the buffer used. He probably has a very good reason to increase the buffer size. In Java you probably want to use a FileChannel directly (get that buffer as close to the OS as possible) and use a 64k byte buffer. \$\endgroup\$ – markspace Nov 5 '19 at 15:29
  • \$\begingroup\$ Ok, thanks for your advices! \$\endgroup\$ – Khaled Nov 6 '19 at 12:25
1
\$\begingroup\$

Improvements:

  • Java 7 try-with-resources handles closing of all that IO in normal and Exceptional circumstances
  • Skip all lines until i'th. Original code loaded the whole file into memory for no benefit.
  • Safely handle small file (readLine() will return null after the last line)
  • Safely handle short line (vals array may be shorter than j)
  • Written as a utility method (better for unit testing)
  • NOT DONE. Many will say i and j should be named more descriptively. E.g. row and col
  • NOT DONE. Exception handling. Maybe you prefer to return null if the file is missing.
  • NOT DONE. Some level of Javadoc
public static String getField(int i, int j, String filename) throws IOException {
    Path path = Paths.get(filename);
    // All these resources will auto-close as we exit the try {} block
    try (BufferedReader reader = new BufferedReader(new InputStreamReader(Files.newInputStream(path)))) {
      String line;
      while ((line = reader.readLine()) != null) { // Read and discard, keeping only current line
        if (i-- == 0) {
          // We made it to the i'th line... 
          String[] vals = line.split(","); // ... Check if we have a j'th field
          return vals.length > j ? vals[j] : null;
        }
      }
      // File finished before i'th line was met
      return null;
    }
  }

For kicks, here's a Java 8 version using Stream and Optional

  public static Optional<String> getFieldJava8(int i, int j, String filename) throws IOException {
    return Files.lines(Paths.get(filename))
      .skip(i)
      .findFirst() // This will output the i'th line
      .flatMap(row -> {
        String[] vals = row.split(","); // check if we have a j'th field
        return vals.length > j ? Optional.of(vals[j]) : Optional.empty();
      });
  }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.