Parsing a tabular log file into a hashmap

I have this method that reads a file into a hashmap. I feel like there are too many levels in this code with all these tries, ifs and so on. How do you think this can be improved?

```
public static ExchangeHistory loadHistory(String path){
Map rtcHistory = new HashMap();
Map almHistory = new HashMap();

ExchangeHistory exchHist = new ExchangeHistory(rtcHistory, almHistory);

File file = new File(path);

BufferedReader reader = null;
try{
reader = new BufferedReader(new InputStreamReader(new FileInputStream(file)));

String line;
int lineNumber = 1;

// read file line by line
while ((line = reader.readLine()) != null){
String[] tokens = line.split("\\s");
if (tokens.length != 3){
log.error("Unable to parse exchange history file at line: " + lineNumber);
continue;
}

// try to parse line. if parsing fails, just skip it
try {
int id = Integer.parseInt(tokens[1]);
long timeInMillis = Long.parseLong(tokens[2]);

if (tokens[0].equals(System.RTC)){
rtcHistory.put(id, new Date(timeInMillis));
} else if (tokens[0].equals(System.ALM)){
almHistory.put(id, new Date(timeInMillis));
}

} catch (Exception e){
log.error("Unable to parse exchange history file at line:" + lineNumber, e);
}

lineNumber++;
}

} catch (FileNotFoundException e){ // try to create a new file in case it doesn't exist

log.warn("No exchange history file found at " + path);

try {
file.createNewFile();
log.info("Created new exchange history file at " + path);
} catch (IOException ioe){
log.error("Unable to create exchange history file at " + path
+ ". Exchange session info won't be saved

This is indeed way too complex. In fact I think there's enough responsibilities in this one method to be spread over a few classes.

• managing the history file

• parsing history records

• processing history records

• counting lines

• logging

These points may map to simply a helper method or a separate class.

Try organizing this behavior in several levels of abstraction. Try checking the existence of the file prior to tryig to parse its contents. Making a class that models the content of one record will definitely help. It'll make a good home for some of the validation and parsing logic. Don't be afraid to make private helper methods. They can greatly improve readability and lower complexity per method significantly.

The top level version of this method could look like this :

public ExchangeHistory loadHistory(String path) {
    List records = new ArrayList<>();
    for (String line : new LineIterable(getHistoryReader(path))) {
        records.add(HistoryRecord.fromTokenizedString(line));
    }
    return ExchangeHistory.fromHistoryRecords(nonNulls(records));
}

In which

• getHistoryReader(path) will fetch or create the file and wrap it as a reader.

• LineIterable abstracts reading the BufferedReader to an Iterable (enabling the advanced for loop)

• HistoryRecord parses the tokens of a line, and can be queried for the parsed data.

• fromTokenizedString(line) returns a valid HistoryRecord or null

• nonNulls() filters nulls out of a List

• ExchangeHistory.fromHistoryRecords() creates the ExchangeHistory based on a List/Iterable of HistoryRecords

As for logging, it always clutters code. Your error logging currently seems to be a placeholder for decent exception handling. The other logging appears to be used for debugging. I like AOP to do logging, since it allows you to focus on the code, and the logging can be coded separately.

StackExchange Code Review Q#26752, answer score: 6