Java exporting filtered objects to CSV

Question

I am exporting a List of virtual servers to a CSV file. The users submits filters and the applications generates FilterDTOs containing a map of the attribute and the corresponding value. The idea was to 'undock' the filter and the simple DTOs that contained all the variables from the export, so the export does not need to know the concrete filter (actually he knows but only to let the FilterDTOs be generated). All values in the map are Optional<String>. Here is the exporter, the getFileEnding and getMediaType are given by the interface to dynamically handle the return of a byte[] in a REST-controller (I need the file ending to parse the file and the media type for setting the HTTP headers). I would appreciate any critic and tips

public class FilteredCsvExporter implements Exporter {
    private ExportFilter filter;

    public FilteredCsvLbExporter(ExportFilter filter) {
        this.filter = filter;
    }

    @Override
    public void exportData() throws IOException, DocumentException {
        List<VirtualServerFilterDTO> servers = PoolRepository.getInstance().getVirtualServerFilterDTOs(filter);
        PrintWriter pw = new PrintWriter(new File(getBaseTempDir() + "/filteredExport.csv"));
        writeDataToFile(pw, new StringBuilder(), servers);
    }

    private void writeDataToFile(PrintWriter printWriter, StringBuilder builder, List<VirtualServerFilterDTO> servers) {
        generateHeadline(builder, servers);
        servers = servers.stream().filter(i -> i.getAttributes().get("Pool").isPresent() != false).collect(Collectors.toList());
        servers.forEach(server -> {
            addMapToCsv(builder, server.getAttributes());
            if (server.getAssignedPool().get().getAttributes().size() <= 0) {
                return;
            }
            addMapToCsv(builder, server.getAssignedPool().get().getAttributes());
            server.getAssignedPool().get().getMembers().forEach(member -> {
                addMapToCsv(builder, member.getAttributes());
            });
            builder.append("\n");
        });
        builder.append("\n");
        builder.append("\n");
        builder.append("Pools that are not assigned to any virtual server");
        builder.append("\n");
        appendPoolsWithoutServerHeader(builder, servers);
        builder.append("\n");
        getPoolsWithoutServer().forEach(pool -> {
            addMapToCsv(builder, pool.getAttributes());
            builder.append("\n");
        });
        printWriter.write(builder.toString());
        printWriter.close();
        System.out.println("Done");
        getPoolsWithoutServer();
    }

    private void appendPoolsWithoutServerHeader(StringBuilder builder, List<VirtualServerFilterDTO> servers) {
        servers.get(0).getAssignedPool().get().getAttributes().forEach((k, v) -> {
            builder.append(k);
            builder.append(",");
        });
    }

    private void addMapToCsv(StringBuilder builder, Map<String, Optional<String>> map) {
        for (Map.Entry<String, Optional<String>> entry : map.entrySet()) {
            if (entry.getValue().orElse("-").contains(",")) {
                builder.append(escapeStringForCSV(entry.getValue()));
                builder.append(",");
            } else {
                builder.append(entry.getValue().orElse("-"));
                builder.append(",");
            }
        }
    }

    private List<PoolFilterDto> getPoolsWithoutServer() {
        List<VirtualServerFilterDTO> servers = PoolRepository.getInstance().getVirtualServerFilterDTOs(filter);
        List<PoolFilterDto> pools = new ArrayList<>();
        List<PoolDTO> poolDTOS = PoolRepository.getInstance().getPools().getPools();
        List<PoolDTO> temp = new ArrayList<>();

        for (int i = 0; i < servers.size(); i++) {
            for (int j = 0; j < poolDTOS.size(); j++) {
                VirtualServerFilterDTO server = servers.get(i);
                PoolDTO pool = poolDTOS.get(j);
                if (server.getAssignedPool().isPresent()) {
                    if (server.getAssignedPool().get().getAttributes().size() > 0) {
                        if (server.getAssignedPool().get().getAttributes().get("Full path").get().equalsIgnoreCase(pool.getFullPath())) {
                            temp.add(pool);
                        }
                    }
                }
            }
        }
        poolDTOS.removeAll(temp);
        poolDTOS.forEach(p -> {
            pools.add(new PoolFilterDto(p, filter));
        });
        return pools;
    }

    private String escapeStringForCSV(Optional<String> value) {
        if (!value.isPresent()) {
            return "-";
        }
        return "\"" + value.get() + "\"";
    }

    private void generateHeadline(StringBuilder builder, List<VirtualServerFilterDTO> servers) {
        servers.get(0).getAttributes().forEach((k, v) -> {
            builder.append(k);
            builder.append(",");
        });
        servers.get(0).getAssignedPool().get().getAttributes().forEach((k, v) -> {
            builder.append(k);
            builder.append(",");
        });

        int size = servers.stream().mapToInt(m -> m.getAssignedPool().get().getMembers().size()).max().getAsInt();

        Map<String, Optional<String>> map = servers.get(0).getAssignedPool().get().getMembers().get(0).getAttributes();
        for (int i = 0; i < size; i++) {
            for (Map.Entry<String, Optional<String>> entry : map.entrySet()) {
                builder.append(entry.getKey());
                builder.append(",");
            }
        }
        builder.append("\n");
    }

    @Override
    public File getBaseTempDir() {
        return new File(System.getProperty("java.io.tmpdir"));
    }

    @Override
    public String getFileEnding() {
        return ".csv";
    }

    @Override
    public String getMediaType() {
        return "text/csv";
    }
}

Answer 1

(Re)use libraries

My main feedback would be that you should not create CSV yourself, but rather use OpenCSV or SuperCSV or another well supported CSV library. If you really want to implement it yourself, make sure it follow the standard: RFC-4180, so you have proper quoting and escaping.

When writing files, check the encoding

You currently use the default character encoding. While this can be correct, oft-times it is better to explicitly choose your own character encoding, for example UTF-8.

Prefer a logging framework for logging instead of System.out

I saw:

 System.out.println("Done");

See here: https://stackoverflow.com/a/8601972/461499

System.out.println is an IO-operation and therefor is time consuming. The Problem with using it in your code is, that your program will wait until the println has finished. This may not be a problem with small sites but as soon as you get load or many iterations, you'll feel the pain.

The better approach is to use a logging framework. They use a message queue and write only if no other output is going on.

Readability of streams

This is a taste-thing, but I prefer to write streams formatted like this:

servers = servers.stream()
                 .filter(i -> i.getAttributes().get("Pool").isPresent() != false)
                 .collect(Collectors.toList());

Also, you can simplify the filter (true != false -> true):

servers = servers.stream()
                 .filter(i -> i.getAttributes().get("Pool").isPresent())
                 .collect(Collectors.toList());

The other thing in streams is something I try to stick to: don't let the stream modify it's outside state. You use some forEach() that do modify the builder. I'd use a plain enhanced for-loop, I find it easier to read and easier to understand.

Don't close when you did not open

writeDataToFile closes a stream that it did not open. This makes it harder to understand, and increases the probability that something goes wrong.

I think the exportData() that creates the PrintWriter should also close it, and catch the appropriate exceptions. Maybe rethow them as needed as different exception. If you use try-with-resources it automatically closes the AutoClosable.

...
         try(PrintWriter pw = new PrintWriter(new File(getBaseTempDir() + "/filteredExport.csv")))
         {
            ...
         } catch (...) { }