Initializing empty instance variables in constructor

Question

I have a LogAnalyzer class that looks at a web server log, creates LogEntry objects and puts those objects into HashMaps for analyzing.

My LogAnalyzer class has these fields:

private int totalVisits;
private int uniqueVisits;
private ArrayList<LogEntry> records;
private HashMap<String, ArrayList<LogEntry>> uniqueIPs; //<address, log entries>
private HashMap<String, ArrayList<LogEntry>> dailyRecords; // <date, log entries>

My constructor looks like this:

public LogAnalyzer() {

    records = new ArrayList<>();
    dailyRecords = new HashMap<>();
    uniqueIPs  = new HashMap<>();

}

And then I have this method:

public void initializeRecords(String path){
    readFile(path); //reads the web log file and fills out the records and dailyRecords fields
    getUniqueIPs(); //fills out the uniqueIPs HashMap field.
    uniqueVisits = uniqueIPs.size(); //fills out the uniqueVisits field
    totalVisits = records.size(); //fills out the totalVisits field
}

So my question:

I have read (but don't really understand) it's "bad" to call methods inside the constructor. However it seems like the constructor is pointless here, since it is actually the initializeRecords that is doing all of the meaningful work of "creating" the object.

I don't have the background in Java or programming to understand the explanations I've found so far. There is a lot of talk of overriding things, and I think that's what I'm not clear on. I'm wondering why I should keep my constructor and this method seperated, in simple terms that a beginner can understand.

**EDIT: ** Here's the code for readFile():

public void readFile(String filename) {
    FileResource fr = new FileResource(filename);
    for (String line : fr.lines()){
        LogEntry le = WebLogParser.parseEntry(line);
        String date = le.getAccessTime().toString().substring(4, 10);
        if (dailyRecords.keySet().contains(date)){
            dailyRecords.get(date).add(le);
        }
        else{
            ArrayList<LogEntry> entries = new ArrayList<>();
            entries.add(le);
            dailyRecords.put(date, entries);
        }
        records.add(le); 
    }

Answer 1

It's not a good practice to call methods from within constructor, because Java always calls the most derived method, which means we could call a method on a half-initialized object.

To answer your question,

public LogAnalyzer() {

    records = new ArrayList<>();
    dailyRecords = new HashMap<>();
    uniqueIPs  = new HashMap<>();

}

What the above part exactly does is, it gives the variables, records, dailyRecods and uniqueIPs a physical address in the memory stack.

When we write something like private ArrayList<LogEntry> records; in the class, at this time only a reference is generated, but actual initialization happens only when records = new ArrayList<>(); this line gets executed.

Hope this clarifies your doubt!!!

Answer 2

As you can see in readFile() it uses the following instruction

dailyRecords.keySet().contains(date)

without initializing dailyRecords prior to it. So if you do not initialize dailyRecords either while declaring it or in constructor you will face NullPointerException.

In your case instead of using constructor for initialization you can use declaration part like this

private HashMap<String, ArrayList<LogEntry>> dailyRecords = new HashMap<>();    

Answer 3

Keeping the two methods allows you more flexibility in the use of your code. You can instantiate your LogAnalyzer without knowing the path to your log. I would rename initializeRecords to processRecords which IMO is more descriptive of what you are doing there.

We should create the object first and then call methods on it. If your readFile method were to throw an Exception because it can't read the file for example. I would find it very odd to get that exception when I am constructing the object. The point of the constructor is to provide an object that can be used to do something.