Using streams for parsing string from file

I learn how to use java 8 API. I have a simple log file with the following contents:

SVF2018-05-24_12:02:58.917
NHR2018-05-24_12:02:49.914
FAM2018-05-24_12:13:04.512
KRF2018-05-24_12:03:01.250
SVM2018-05-24_12:18:37.735
MES2018-05-24_12:04:45.513
LSW2018-05-24_12:06:13.511
BHS2018-05-24_12:14:51.985
EOF2018-05-24_12:17:58.810
RGH2018-05-24_12:05:14.511
SSW2018-05-24_12:16:11.648
KMH2018-05-24_12:02:51.003
PGS2018-05-24_12:07:23.645
CSR2018-05-24_12:03:15.145
SPF2018-05-24_12:12:01.035
DRR2018-05-24_12:14:12.054
LHM2018-05-24_12:18:20.125
CLS2018-05-24_12:09:41.921
VBM2018-05-24_12:00:00.000

My goal is to parse it using streams. The desired output is the following:

[{SVF = [2018-05-24, 12:02:58.917]}, {NHR = [2018-05-24, 12:02:49.914]}...]

I already have the following:

public class FileParser {
	Stream<String> outputStream;
	public FileParser(String fileName) throws IOException, URISyntaxException {
		FileReader fr = new FileReader();
		this.outputStream = fr.getStreamFromFile(fileName);

    public List<HashMap<String,ArrayList<String>>> getRacersInfo(){
	    return outputStream.map(line -> Arrays.asList(line.substring(0,3))
			    .collect(Collectors.toMap(???)); //Some code here which I cannot come up with.
    }

Any help appreciated. If you need any additional information feel free to ask, I'll be glad to provide it.

Problem: FileReader

FileReader is obsolete, don't use it. It's outdated API, and it's problematic, in that it presumes 'platform default encoding' which is a different way of saying 'a bug waiting to happen that no test will catch but that will blow up in your face later'. You never want 'platform default encoding', especially as a silent default.

There's a new File API, and it lets you specify encoding explicitly. Also, in the new File API, if you don't, UTF-8 is assumed which is a far saner default than 'platform default'.

Problem: resources

Resources are objects that represent a resource that takes up OS-level handles. Files, network connections, database connections - those are some common examples of resources. The thing is unlike normal objects, you MUST explicitly CLOSE those. - if you don't, your VM will, eventually, crash. That means you can basically not put readers/inputstreams/outputstreams/writers in fields, ever, because how do you guarantee closing them? The only way is to make your own class a resource too (a thing that must explicitly be closed), which you can do, but is complicated, and not a good idea here.

You should never make resources unless you do so safely:

bad:

FileReader fr = new FileReader(..);

good:

try (FileReader fr = new FileReader(..)) {
    // use here
}
// it's gone here

It does require you to restyle things a bit. You have to open a resource, use the resource, and close it. This meshes well with pragmatic concerns: Resources are a drain on the OS, you don't want to keep em open any longer than you must, so 'open it, use it, and lose it' is the right mindset.

Furthermore, of course, resources as a concept are generally 'once-through-only'. for example, when reading a file, well, you read it, once, from the top to the bottom, and then any further attempts to read from it don't work anymore. So, in your example, the first time I call getRacersInfo(), it works. But the second time I call it, it won't, as the reader has now been consumed.

The solution to both problems is to do the reading in the constructor*.

*) See later - we're going to move this out of the constructor eventually, but that's a separate concern.

Problem: Misunderstanding of responsibilities of constructors

This class is called a FileParser. So, it's job is to parse files (that, or, this class has a bad name). Generally, your constructors represent the 'data gathering' phase, not the 'do the job' phase. Therefore, parsing the file in the first place, in that constructor, is bad code style. You should not do this - your constructors should as a rule do as little as possible and definitely nothing tricky, such as opening files or actually parsing things. Again - the JOB of a FileParser is to parse files, and constructors should not do the job. They just set up the object so that it can do the job later.

The proper design, then, is:

public class FileParser {
    private final Path path;

    public FileParser(Path path) {
        this.path = path;
    }

    public List<Map<String, List<String>> parseRacersInfo() {
        try (Stream<String> lines = Files.lines(path)) {
            lines.map(.... parse the content here ....);
        }
    }
}

We have now:

1. Moved the 'job' part to a method that accurately describes the job.
2. Ensured the constructor is simple and just gathers information to do the job.
3. Safely use resources by applying the try(){} concept.
4. Use the new API (java.nio.file.Files and java.nio.file.Path).
5. Clarified our typing: That parameter to the constructor represents a path. If I call new FileParser("Hello, IceTea, how's life?") - that call makes no sense. Path is more descriptive than String, and if your method makes sense looking only at the types of the parameters? That's better than if you need to read the docs too.

Problem: Not using java the way it wants to be used

Java is typed. Nominally so. Things should be stored in types that represent that thing. Thus, the string 2018-05-24_12:18:20.125 should be represented by an object that represents a time of some sort. Not a List<String> containing the string 2018-05-24 and 12:18:20.125.

Finally: How do I actually write the mapping?

Streams work by zooming in on a single element in the stream, and doing a series of operations on these elements, transforming them, filtering some out, etcetera. You cannot 'go back' in the process (once you map a thing to another thing, you can't go back to what it used to be), and you can't refer to other objects in your stream (you can't ask: Give me the item before me in the stream).

Thus, once you go: line.substring(0, 3), you've thrown out the date, and that's a problem because we need that info. Therefore, you can't do that; not in a .map() operation, at any rate.

In fact, we can go straight to collecting the stream back into a map here - we need that entire string and we can derive the key from it (SVF), and we need that entire string and we can derive the value from it (the date).

Let's write these conversion functions, and let's translate our string representing a time to a proper (also new in java 8) type for it: java.time.LocalDateTime:

Function<String, String> toKey = in -> in.substring(0, 3);

DateTimeFormatter DATETIME_FORMAT =
  DateTimeFormatter.ofPattern("uuuu-MM-dd_HH:mm:ss.SSS", Locale.ENGLISH);
Function<String, LocalDateTime> toValue = in -> 
    LocalDateTime.parse(in.substring(3), DATETIME_FORMAT);

These are simple and we can test them:

assertEquals("VBM", toKey.apply("VBM2018-05-24_12:00:00.000"));
assertEquals(LocalDateTime.of(2018, 5, 24, 12, 0, 0),
    toValue.apply("VBM2018-05-24_12:00:00.000"));

Then we put it all together:

import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.Files;
import java.io.IOException;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class FileParser {
    private static final DateTimeFormatter DATETIME_FORMAT =
        DateTimeFormatter.ofPattern("uuuu-MM-dd_HH:mm:ss.SSS", Locale.ENGLISH);

    private final Path path;

    public FileParser(Path path) {
        this.path = path;
    }

    public Map<String, LocalDateTime> parseRacersInfo() throws IOException {
        try (Stream<String> lines = Files.lines(path)) {
            return lines.collect(Collectors.toMap(
                in -> in.substring(0, 3),
                in -> LocalDateTime.parse(in.substring(3), DATETIME_FORMAT)));
        }
    }

    public static void main(String[] args) throws Exception {
        System.out.println(new FileParser("test.txt").parseRacersInfo());
    }
}

Path path = Paths.get(fileName);
try (Stream<String> lines = Files.lines(path)) {
    Map<String, List<LocalDateTime>> map = lines.collect(Collectors.groupingBy(
        line -> line.substring(0, 3),
        line -> LocalDateTime.parse(line.substring(3).replace('_', 'T')));
}

The toMap receives a key mapper and a value mapper. Here I keep the Stream of lines.

The resulting map is just a Map. Never provide an implementation, HashMap so the collect may return its own implementation. (If effect you could provide an implementation.)

(I used Files.lines which defaults to UTF-8 encoding, but you can add an encoding. The reason: Path is more generalized than File.)

Something like :

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class Test {

    public static void main(String[] args) {
        String fileName = "C:\\Users\\Asmir\\Desktop\\input1.txt";
        Map<String,List<String>> map = new HashMap<>();

        try (Stream<String> stream = Files.lines(Paths.get(fileName))) {
            map = stream
                    .collect(Collectors.toMap(s -> s.substring(0,3), s -> Arrays.asList(s.substring(3).split("_"))));

        } catch (IOException e) {
            e.printStackTrace();
        }
        System.out.println(map);
    }
}