11

I am trying to learn how to utilize Java 8 features(such as lambdas and streams) in my daily programming, since it makes for much cleaner code.

Here's what I am currently working on: I get a string stream from a local file with some data which I turn into objects later. The input file structure looks something like this:

Airport name; Country; Continent; some number;

And my code looks like this:

public class AirportConsumer implements AirportAPI {

List<Airport> airports = new ArrayList<Airport>();

@Override
public Stream<Airport> getAirports() {
    Stream<String> stream = null;
    try {
        stream = Files.lines(Paths.get("resources/planes.txt"));
        stream.forEach(line -> createAirport(line));

    } catch (IOException e) {
        e.printStackTrace();
    }
    return airports.stream();
}

public void createAirport(String line) {
    String airport, country, continent;
    int length;


    airport = line.substring(0, line.indexOf(';')).trim();
    line = line.replace(airport + ";", "");
    country = line.substring(0,line.indexOf(';')).trim();
    line = line.replace(country + ";", "");
    continent = line.substring(0,line.indexOf(';')).trim();
    line = line.replace(continent + ";", "");
    length = Integer.parseInt(line.substring(0,line.indexOf(';')).trim());
    airports.add(new Airport(airport, country, continent, length));
    }
}

And in my main class I iterate over the object stream and print out the results:

public class Main {



public void toString(Airport t){
  System.out.println(t.getName() + " " + t.getContinent());
}

public static void main(String[] args) throws IOException {
    Main m = new Main();
    m.whatever();

}

private void whatever() throws IOException {
    AirportAPI k = new AirportConsumer();
    Stream<Airport> s;
    s = k.getAirports();
    s.forEach(this::toString);

}


}

My question is this: How can I optimize this code, so I don't have to parse the lines from the file separately, but instead create a stream of objects Airport straight from the source file? Or is this the extent in which I can do this?

1
  • 3
    Note: you should close the file: try(Stream<String> lines = Files.lines(...)) { lines.map(xxx).collect(...) }; Commented May 27, 2015 at 12:20

2 Answers 2

13

You need to use map() to transform the data as it comes past.

Files.lines(Paths.get("resources/planes.txt"))
     .map(line -> createAirport(line));

This will return a Stream<Airport> - if you want to return a List, then you'll need to use the collect method at the end.

This approach is also stateless, which means you won't need the instance-level airports value.

You'll need to update your createAirport method to return something:

public Airport createAirport(String line) {

    String airport = line.substring(0, line.indexOf(';')).trim();
    line = line.replace(airport + ";", "");
    String country = line.substring(0,line.indexOf(';')).trim();
    line = line.replace(country + ";", "");
    String continent = line.substring(0,line.indexOf(';')).trim();
    line = line.replace(continent + ";", "");
    int length = Integer.parseInt(line.substring(0,line.indexOf(';')).trim());
    return new Airport(airport, country, continent, length);
}

If you're looking for a more functional approach to your code, you may want to consider a rewrite of createAirport so it doesn't mutate line. Builders are also nice for this kind of thing.

public Airport createAirport(final String line) {
    final String[] fields = line.split(";");
    return new Airport(fields[0].trim(), 
                       fields[1].trim(), 
                       fields[2].trim(), 
                       Integer.parseInt(fields[3].trim()));
}

Throwing it all together, your class now looks like this.

public class AirportConsumer implements AirportAPI {

    @Override
    public Stream<Airport> getAirports() {
        Stream<String> stream = null;
        try {
            stream = Files.lines(Paths.get("resources/planes.txt"))
                                   .map(line -> createAirport(line));
        } catch (IOException e) {
            stream = Stream.empty();
            e.printStackTrace();
        }
        return stream;
    }

    private Airport createAirport(final String line) {
        final String[] fields = line.split(";");
        return new Airport(fields[0].trim(), 
                           fields[1].trim(), 
                           fields[2].trim(), 
                           Integer.parseInt(fields[3].trim()));
    }
}
Sign up to request clarification or add additional context in comments.

8 Comments

Provided that the createAirport method is changed to return the created airport instead of appending it to that (now obsolete) list.
As an addition I would recommend to read the java.util.stream package description. It's not very long and really helpful to understand the streams.
I'm talking about mutation of the reference. The line passed in does not have the same value as line half-way down the method.
Your try(…) statement will close the stream before it is returned, that’s surely not what you want. The try-with-resources construct is only useful for resources you are processing within the scope of the statement, not when you want to return the resource to the method’s caller. Btw., I don’t get why you name a class which produces Airport instances AirportConsumer.
But that’s not how streams work. All intermediate operations are lazy and are performed when the sink (aka terminal operation) requires another element. Hence, the last line will be mapped to an Airport instance right before the terminal operation processes the last element (if it ever does, as it might be short-circuiting). So there is no place where all lines have been converted already but keeping the stream open would make sense.
|
0

The code posted by Steve looks great. But there are still two places can be improved: 1, How to split a string. 2, It may cause issue if the people forget or don't know to close the stream created by calling getAirports() method. So it's better to finish the task(toList() or whatever) in place. Here is code by abacus-common

try(Reader reader = IOUtil.createBufferedReader(file)) {
    List<Airport> airportList = Stream.of(reader).map(line -> {
        String[] strs = Splitter.with(";").trim(true).splitToArray(line);
        return Airport(strs[0], strs[1], strs[2], Integer.valueOf(strs[3]));
    }).toList();
} catch (IOException e) {
    throw new RuntimeException(e);
}

// Or By the Try:

List<Airport> airportList = Try.stream(file).call(s -> s.map(line -> {
    String[] strs = Splitter.with(";").trim(true).splitToArray(line);
    return Airport(strs[0], strs[1], strs[2], Integer.valueOf(strs[3]));
}).toList())

Disclosure: I'm the developer of abacus-common.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.