Refactoring csv reading, parsing and object initializing code snippet

Question

I try to refactor some old code which I use to read data from CSV file, parse every line and initialize an object instance for every line in the CSV file and save every object in a list.
I also try learn S.O.L.I.D principles and apply them.

As for Single responsibility principle I have a class CsvReader and it has a method read().
I think that this method has too many responsibilities like
reading a line (1),
parsing it (2),
initializing a new object instance of Row (3)
and finally storing all of them in a list (4).

I also don't like that I am newing it up in the read method (5).

How should I refactor it? Are these 5 points real problems or am I worring too much about it and shouldn't try to follow SRP everywhere. Seems it isn't reasonable to use it everywhere.

public void read() {

    String thisLine = null;
    String[] splitted = null;
    List<Row> rows = new ArrayList<>();

    try {
        BufferedReader br = new BufferedReader(new FileReader(new File(
                "input.txt")));
        while ((thisLine = br.readLine()) != null) {
            splitted = thisLine.split(",");
            Row row = new Row(splitted[0], splitted[1]);
            rows.add(row);
        }
        br.close();
    } catch (Exception e) {
        e.printStackTrace();
    } 
}

However, I am not sure if the following violates SRP. I belive that newing it up is additional responsibility.

Row = new Row(splitted[0],splitted[1]);

This is how I would refactor it myself. I would put line splitting into a separate class named Parser. But I am not sure how Parser should interact. Should Parser itself initialize a Row instance and return it? But then again I have separated line splitting into Parser class but it seems that I am still stuck because Parser class must do splitting and also newing up a Row instance. This is my refactored code (CsvReader class that has only one method named read()):

public void read(List<Row> row) {

    String thisLine = null;

    try {
        BufferedReader br = new BufferedReader(new FileReader(new File(
                "input.txt")));
        while ((thisLine = br.readLine()) != null) {
            //splitted = thisLine.split(",");
            Row row = Parser.parse(thisLine);
            //Row row = new Row(splitted[0], splitted[1]);
            rows.add(row);
        }
        br.close();
    } catch (Exception e) {
        e.printStackTrace();
    } 
}

And Parser class would be:

public class Parser {

    static String[] splitted = null;

    static Row parse(String inputLine) {
        splitted = inputLine.split(",");
        return new Row(splitted[0], splitted[1]);
    }

}

I think that static Parser.parse method should only parse and not new up an instance of Row class.

Answer 1

Could be something like this.

One class for parsing the file:

public class Parser {

    private final String file;
    private final LineParser lineParser;

    public Parser(final String file) {
        this.file = file;
        lineParser = new LineParser();
    }

    public ArrayList<Row> parse() {
        String thisLine = null;
        ArrayList<Row> rows=new ArrayList<Row>();
        try {
            BufferedReader br = new BufferedReader(new FileReader(file));
            while ((thisLine = br.readLine()) != null) {
                Row row = lineParser.parse(thisLine);
                rows.add(row);
            }
            br.close();
        } catch (Exception e) {
            e.printStackTrace();
        }
        return rows;
    }
}

Other class for parsing the line:

public class LineParser {
    public Row parse(final String line) {
        String[] splitted = line.split(",");
        return new Row(splitted[0], splitted[1]);
    }
}

Try to avoid static methods.

Answer 2

1. There is no reason to create a Parser class just to define the parse() method. Put that parse() method in the CsvReader class. It's the purpose of pricate methods: to be used internally by others methods of the class.
2. Then a good practice is to provide an InputStream as the parameter for the read() method. That way if in the future you want to use the method to read something else that a file (a URL or a socket for example), there is nothing to change.
3. Can I suggest your read method to throw an exception? If something wrong happens during the operation, it's better to be notified. Using an exception, you can do it at a higher level (in the GUI for example).

4. Finally, you should rely on the AutoCloseable mecanism when you open your Reader.

   public class CsvReader {
   
       public List<Row> read(final InputStream in) throws IOException {
   
           final List<Row> rows = new ArrayList<>();
           try (final BufferedReader br = new BufferedReader(
                   new InputStreamReader(in))) {
               String line = br.readLine();
               while (line != null) {
                   rows.add(parse(line));
                   line = br.readLine();
               }
           }
           return rows;
       }
   
       private Row parse(final String inputLine) {
           final String[] splitted = inputLine.split(",");
           return new Row(splitted[0], splitted[1]);
       }
   }
   

As a side note: do you really need to use a Row class to store your data? A String array would work perfectly here.