0
\$\begingroup\$

I want to have 2 classes responsible for reading dictionaries from CSV.
The 1st class takes 2 columns from CSV and makes Map<String, String>.
The 2nd class takes 2 columns from CSV and makes Map<String, Double>.

I want to solve it according to DRY rule so I've made a generic method for making Map<T1, T2> from csv which will be used by 1st class and 2nd class. It goes like this:

package Main;

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 MapFromCsv<T1, T2> {

    public default Map<T1, T2> getMap(String FILE_TO_LOAD, Function<String, T1> foo1, Function<String, T2> foo2){
        Map<T1, T2> 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((T1) foo1.apply(tmp2[0]), (T2) foo2.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;
    }
}

and implementation of the first class for making Map<String, String> goes like this:

package Main;

import java.util.Map;

public enum StringMapFromCsv implements MapFromCsv<String, String> {

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

    private StringMapFromCsv() {
        initStringMap();
    }

    private void initStringMap() {
        this.stringMap = getMap(FILE_TO_LOAD);
    }

    public void reloadStringMap() {
        initStringMap();
    }

    public Map<String, String> getStringMap() {
        return stringMap;
    }

}

and implementation of the second class for making Map<String, Double> goes like this:

package Main;

import java.util.Map;

public enum DoubleMapFromCsv implements MapFromCsv<String, Double>{

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

    private DoubleMapFromCsv() {
        initDoubleMap();
    }

    private void initDoubleMap() {
        this.doubleMap = getMap(FILE_TO_LOAD);
    }

    public void reloadDoubleMap() {
        initDoubleMap();
    }

    public Map<String, Double> getDoubleMap() {
        return doubleMap;
    }

}

Anu suggestions?

\$\endgroup\$
2
  • \$\begingroup\$ Any specific reasons you chose to implement with ENUMS not Class ?? \$\endgroup\$
    – Tiny Rick
    Oct 31, 2017 at 7:32
  • \$\begingroup\$ Many other classes will use thos dictionaries many times and reading from csv takes few seconds. \$\endgroup\$
    – wBacz
    Oct 31, 2017 at 7:36

1 Answer 1

1
\$\begingroup\$

This really looks like you over-designed the solution. And especially tricky since it won't work right now. (You're calling an unknown method getMap(FILE_TO_LOAD), it's missing the 2 mapping functions as parameters).

Why not just create a factory class that creates maps from the CSV files?

public class MapFactory {
    private static final String STRING_FILE = "resources/stringMap.csv";
    private static final String DOUBLE_FILE = "resources/doubleMap.csv";

    //private constructor to prevent pointless instance creation of this class
    private MapFactory() {
    }

    private static Map<String, String> createStringMap() {
        return createMapFromFile(STRING_FILE, Function.identity());
    }

    private static Map<String, Double> createDoubleMap() {
        return createMapFromFile(DOUBLE_FILE, Double::parseDouble);
    }

    public static <T> Map<String, T> createMapFromFile(String inputFile, Function<String, T> parseFunction) {
        Map<String, T> mapResult = new HashMap<>();
        File f = new File(inputFile);
        try (Scanner sc = new Scanner(f)) {
            while (sc.hasNextLine()) {
                String line = sc.nextLine();
                String[] lineParts = line.split(";");
                mapResult.put(lineParts[0], parseFunction.apply(lineParts[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;
    }
}

You should really change the names of "stringMap" and "doubleMap" to something more meaningful. I can't do that for you here since you did not provide the actual use of your code.

Notice how I just extracted the common part into it's own method here?

I did throw away the map field so you'll need to re-read the file on each new creation. This greatly depends on how you plan on using these maps. With this solution the different classes that use the maps are free to modify it without breaking any other classes that use the map.

Alternatively you can also save the map in a field and use (thread safe) lazy loading to only initialise the map once, the first time it's used like this:

public class MapFactory {
    private static final String STRING_FILE = "resources/stringMap.csv";
    private static final String DOUBLE_FILE = "resources/doubleMap.csv";

    //private constructor to prevent pointless instance creation of this class
    private MapFactory() {
    }

    private static class StringMapHolder {
        private static final Map<String, String> STRING_MAP =createMapFromFile(STRING_FILE, Function.identity()); 
    }

    public static Map<String, String> createStringMap() {
        return StringMapHolder.STRING_MAP;
    }
    private static class DoubleMapHolder {
        private static final Map<String, Double> DOUBLE_MAP = createMapFromFile(DOUBLE_FILE, Double::parseDouble);

    }

    public static Map<String, Double> createDoubleMap() {
        return DoubleMapHolder.DOUBLE_MAP;
    }

    private static <T> Map<String, T> createMapFromFile(String inputFile, Function<String, T> parseFunction) {
        Map<String, T> mapResult = new HashMap<>();
        File f = new File(inputFile);
        try (Scanner sc = new Scanner(f)) {
            while (sc.hasNextLine()) {
                String line = sc.nextLine();
                String[] lineParts = line.split(";");
                mapResult.put(lineParts[0], parseFunction.apply(lineParts[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;
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ oh, I passed bad code, how it is possible. In StringMap class I wanted write getMap(FILE_TO_LOAD, (s) -> s) and in the DoubleMap getMap(FILE_TO_LOAD, Double::parseInt). I think your solution with factory is the best. \$\endgroup\$
    – wBacz
    Oct 31, 2017 at 23:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.