Reading Map from csv with Singleton

Question

There are 2 csv:
1 - areaMap.csv
2 - currencyMap.csv

Enum AreaMapFromCsv is the Singleton to store the Map<String, Enum> read from 1 csv.
Enum CurrencyMapFromCsv is the Singleton to store the Map<String, String> read from 2 csv. Interface IMapFromCsv provides default method for reading Map from csv.

AreaMapFromCsv and CurrencyMapFromCsv looks almost identical. They differs only in FILE_TO_LOAD, argument passing to the interface default method and second type of Map parameters. Any ideas how to DRY?

import java.io.File;
import java.io.FileNotFoundException;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;
import java.util.function.Function;

public interface IMapFromCsv{

    public default <T> Map<String, T> getMap(String FILE_TO_LOAD, Function<String, T> convertValue){
        Map<String, T> mapResult = new HashMap<>();
        File f = new File(FILE_TO_LOAD);
        try (Scanner sc = new Scanner(f)) {
            while (sc.hasNextLine()) {
                String tmp = sc.nextLine();
                String[] tmp2 = tmp.split(";");
                mapResult.put(tmp2[0], convertValue.apply(tmp2[1]));
            }
        } catch (FileNotFoundException fntf) {
            System.err.println("File " + f.getPath() + "was not found.");
        } catch (Exception e) {
            System.err.println("Something went wrong with reading " + f.getPath());
        }
        return mapResult;
    }
}

.

import java.util.Map;
import java.util.function.Function;

public enum AreaMapFromCsv implements IMapFromCsv {

    INSTANCE;
    final String FILE_TO_LOAD = "resources/areaMap.csv";
    private Map<String, String> map;

    private AreaMapFromCsv() {
        init();
    }

    private void init() {
        this.map = getMap(FILE_TO_LOAD, Function.identity());
    }

    public void reload() {
        init();
    }

    public Map<String, String> get() {
        return map;
    }

}

.

import java.util.Map;

public enum CurrencyMapFromCsv implements IMapFromCsv {

    INSTANCE;
    final String FILE_TO_LOAD = "resources/currencyMap.csv";
    private Map<String, Double> map;

    private CurrencyMapFromCsv() {
        init();
    }

    private void init() {
        this.map = getMap(FILE_TO_LOAD, Double::parseDouble);
    }

    public void reload() {
        init();
    }

    public Map<String, Double> get() {
        return map;
    }

}

Answer 1

soo... going from top to bottom:

public interface IMapFromCsv{

superfluous I before name, missing space before opening curly brace

public default <T> Map<String, T> getMap(String FILE_TO_LOAD, Function<String, T> convertValue){

FILE_TO_LOAD should be named according to naming convention: fileToLoad. Next step is making it a Path because a String is not a File and File is superseeded by Path, because it sucked.

Also missing a space before the opening curly again.

    Map<String, T> mapResult = new HashMap<>();
    File f = new File(FILE_TO_LOAD);

so far so good. did I mention that File is crap?

    try (Scanner sc = new Scanner(f)) {
        while (sc.hasNextLine()) {
            String tmp = sc.nextLine();
            String[] tmp2 = tmp.split(";");
            mapResult.put(tmp2[0], convertValue.apply(tmp2[1]));
        }
    }

Bonus points for using try-with-resources, deductions for using tmp and tmp2 as variable names. How about we simplify this a bit?

try (Stream<String> lines = Files.lines(fileToLoad)) {
    return lines.map(s -> s.split(";"))
        .collect(Collectors.toMap(line -> line[0]
            , line -> convertValue.apply(line[1])));
}

Note that this only returns either the full map, or currently nothing. If something goes wrong while reading the map, I'd suggest using Collections.emptyMap() as a sensible default result.

    } catch (FileNotFoundException fntf) {

This exception is one of the reasons File is crap. The good part is: You get a unified Exception API from the java.nio.Path and related, which is: IOException.

---

At this point I'm asking myself the question why this is a default method on an interface. Methods in an interface are meant to be overwritten to modify them. The method instead takes parameters to configure it. IMO this would be much nicer as a static method in some IO module or something.

---

Last but not least: Why are AreaMapFromCsv named like they'd be Methods? Let's reconsider this for a second. You're trying to implement something that will hold information on Maps of your choosing, namely where they are saved, and what value you're currently assigning to them.

I'm not clear on why that needs to be two separate classes (or actually singleton enums), or where the singleton comes into this.

From my PoV it seems like you heard that Singletons are an acceptable solution when you try to handle global state. Tip: they aren't.

The correct solution for global state is a properly encapsulated statically accessible instance of a proper class. Consider this instead:

public class MapHolder<T> {

    // consider making this final
    private Map<String, T> map;
    private final String mapPersistenceLocation; // this is a Path...
    private final Function<String, T> converter;

    public MapHolder(String parameterName, Function<String, T> converter) {
        this.mapPersistenceLocation = parameterName;
        this.converter = converter;
        reload();
    }

    public void reload() {
        // if map is final, replace by map.clear(); map.putAll(...);
        map = IOHelper.readMap(mapPersistenceLocation, converter);
    }

    public Map<String, T> get() {
        // consider returning a readonly copy, if modifications should be disallowed
        return map;
    }
}

Which can now be used as follows:

public final class GlobalContext {
    public static final MapHolder<Double> CURRENCY = new MapHolder<>("resources/currencyMap.csv", Double::parseDouble);
    public static final MapHolder<String> AREA = new MapHolder<>("resources/areaMap.csv", Function.identity());

    // prevent instantiation
    private GlobalContext() {}
}