A Java class for reading MaCH dosage files v2.0

Question

Version 2 of A Java class for reading MaCH dosage files

import java.io.*;
import java.util.ArrayList;
import java.util.regex.Pattern;
import java.util.zip.GZIPInputStream;

/**
 * Created by vu.co.kaiyin.ReadMachDosage on 15/11/14.
 */
public class Dosage {
    final String filename;
    final Pattern splitPattern;

    public Dosage(String filename) throws Exception {
        this.filename = filename;
        splitPattern = Pattern.compile("\\s+");
    }

    public double[][] readCols(int start, int end) throws IOException {
        try(
                InputStream fileStream = new FileInputStream(filename);
                InputStream gzipStream = new GZIPInputStream(fileStream);
                Reader reader = new InputStreamReader(gzipStream, "US-ASCII");
                BufferedReader bufferedReader = new BufferedReader(reader);
                ) {
            String line;
            String[] fields;
            ArrayList<double[]> dosageList = new ArrayList<>();
            boolean firstLine = true;
            int nCol;
            while ((line = bufferedReader.readLine()) != null) {
                fields = splitPattern.split(line);
                if(firstLine) {
                    nCol = fields.length - 2;
//          Data indices: x      x    1      2      3
//               Indices: 0      1    2      3      4
//               Columns: 1->241 DOSE 1.1783 0.3498 1.9834 ...
                    if(start < 1) start = 1;
                    if(end > nCol) end = nCol;
                    start++;
                    end += 2; // java end index is exclusive
                    firstLine = false;
                }
                double[] dosage = new double[end - start];
                for (int i = start; i < end; i++) {
                    dosage[i - start] = Double.parseDouble(fields[i]);
                }
                dosageList.add(dosage);
            }
            double[][] dosageMatrix = new double[dosageList.size()][];
            dosageMatrix = (double[][]) dosageList.toArray(dosageMatrix);
            return dosageMatrix;
        }
    }



    public double[][] readAll() throws Exception {
        return readCols(1, Integer.MAX_VALUE);
    }
}

Changes:

• Read the file only once
• Removing unused fields
• Bug fixes in array elements assignment
• More specific exceptions

Answer 1

I have a number of suggestions:

Exceptions

I don't think your constructor actually throws Exception. Remove it?

Braces in if-blocks

Use braces even for one line if-blocks.

if(start < 1) {
    start = 1;
}
if(end > nCol)  {
    end = nCol;
}

Start parameter adjustment

The adjustment of start can be done before entering the while-loop. Making the while loop shorter and easier to understand. I suggest placing it at the beginning of the function, as you are adjusting an input parameter. Some people think it is bad practice to overwrite a parameter's value, but that depends on your personal taste. I think it is ok in this case, if done at the beginning of the function.

A different way to handle the situation of an incorrect input would be to throw an exception, because clearly the input is incorrect.

Documentation

I'd sugguest adding some comments to at least your public methods, so that the user of your utility function knows how to use it properly. This would also be a good place to document how you handle incorrect input (e.g. the correction of the start value to one, in case the entered value is smaller than 1).

Pattern as static variable

As the used pattern for splitting is a constant, you could handle it as such buy using a static attribute like this:

private static final Pattern splitPattern = Pattern.compile("\\s+");

This saves the jvm from compiling the pattern for each new instance of the Dosage class.

Micro optimization

You can save yourself the firstLine boolean by checking dosageList.isEmpty() instead.