Calling private methods from inside the constructor in Java

Question

I have the following class:

package com.tesco.demandforecasting.group8.choprachap7;

import java.util.ArrayList;

import com.tesco.demandforecasting.group8.utils.MathOperUtils;
import com.tesco.demandforecasting.group8.utils.RegressionUtils;

import lombok.Getter;

/**
 * This class if used to find seasonality factor of each period, as explain in
 * the chapter See https://kelley.iu.edu/mabert/e730/Chopra-Chap-7.pdf for the
 * explanation
 */
@Getter
public class ChopraChap7SeasonalFactorsCalculator {

    private double[] regressionParams;

    private int sales_size;
    private int periodicity;

    private ArrayList<Integer> sales;
    private ArrayList<Double> deseasonalisedData;
    private ArrayList<Double> deseasonalisedDemandUsingRegression;
    private ArrayList<Double> seasonalityFactors;

    public ChopraChap7SeasonalFactorsCalculator() {

        this.sales = new ArrayList<>();
        this.deseasonalisedData = new ArrayList<>();
        this.deseasonalisedDemandUsingRegression = new ArrayList<>();
        this.seasonalityFactors = new ArrayList<>();

        this.sales.add(8000);
        this.sales.add(13000);
        this.sales.add(23000);
        this.sales.add(34000);
        this.sales.add(10000);
        this.sales.add(18000);
        this.sales.add(23000);
        this.sales.add(38000);
        this.sales.add(12000);
        this.sales.add(13000);
        this.sales.add(32000);
        this.sales.add(41000);

        this.sales_size = sales.size();
        this.periodicity = 4;

        calculateSeasonalityFactors();
    }


    private void calculateSeasonalityFactors() {

        .......
        .......

        this.seasonalityFactors = seasonalityFactors;
        this.deseasonalisedDemandUsingRegression = deseasonalisedDemandUsingRegression;
        this.deseasonalisedData = deseasonalisedData;

    }

}

I want to expose the class fields to external classes, using their respective getters. But, the problem is that those fields attain any value only after the ChopraChap7SeasonalFactorsCalculator() method is called. So, what I have done here is call the method as soon as an object of the class is created. Of course, this will work, but is this good design pattern?

Supposing I would not have called the method from the constructor. So, if we have the following code is some other class:

ChopraChap7SeasonalFactorsCalculator calc = new ChopraChap7SeasonalFactorsCalculator();
calc.getDeseasonalisedData();

This will return to me any empty array list. How do I ensure that the method is called before any field is accessed?

What would be the best design pattern in my case?

How's about this `getSales() { if(sales == null){// call here u r method}}` — mallikarjun, Jul 31 '18 at 13:08
Woah man, so beautiful. I read it somewhere that getters are meant for such extra code, and not simply returning the variable's value. Upvoted. Could you also add this as an answer? — Prashant Pandey, Jul 31 '18 at 13:10
Getter should get the current value and not do complex initialization. Imagine you add another method which is for example public calculateSales() which uses the sales field internally without calling the getter - it would fail — Veselin Davidov, Jul 31 '18 at 13:16
There is a similar question; [Can I call methods in constructor in Java?](https://stackoverflow.com/questions/5230565/can-i-call-methods-in-constructor-in-java) The accepted answer there suggests to use the factory pattern. This could be an alternative. — LuCio, Jul 31 '18 at 14:24

davidxxx · Accepted Answer · 2018-07-31T13:16:05.750

Of course, this will work, but is this good design pattern?

This is a very correct design. You delegate a part of the constructor logic into a private method to make things clearer.

This will return to me any empty array list. How do I ensure that the method is called before any field is accessed?

Your fear about someone changes something in the constructor may be true for any methods or chunks of code.
But applications are not designed to check that each component does what we expect from it. This is the unit tests role to assert that the actual behavior is which one expected.

So write an unit test for the ChopraChap7SeasonalFactorsCalculator constructor and in this test assert that all getters return the expected values once the object is created.
If someone modifies the constructor in an incorrect way, the test will fail and the build too. You have your way to make sure things are as expected now.

It should be noted that a constructor should not call methods that can be overridden (e.g. methods that are not private and not marked final or within a final class). If a subclass overrides this method, it could access fields of the object before they are fully constructed (stable). — Justin Albano, Jul 31 '18 at 15:38
@Justin Albano I completely agree about this point. It is not explicitly stated but that is the reason why I specified `private` method. — davidxxx, Jul 31 '18 at 15:41

Veselin Davidov · Answer 2 · 2018-07-31T13:12:39.117

1

I think that's pretty fine. The constructor is there to create a useful object. If you are sure the object cannot be used without these being set there is no reason why not to set them in the constructor.

If you check https://docs.oracle.com/javase/tutorial/java/javaOO/constructors.html

A class contains constructors that are invoked to create objects from the class blueprint.

You have added the fields but you don't have a working object without these being set and apparently you know the values already. The best way to do it would be leave these in the constructor. If there are some unknown values or requirements in order to create an instance of that class you can consider Factory pattern or something but in your case constructor usage is just fine.

edited Jul 31 '18 at 13:12

answered Jul 31 '18 at 13:09

Veselin Davidov

7,031
1
15
23

Also, I could check for empty or null fields in the getter itself, as @malikarjun suggested. – Prashant Pandey Jul 31 '18 at 13:11
1

Yeah but that would kind of break the SOLID principle in OOP I think. Getter should get the current value and not do complex initialization. Imagine you add another method which is for example calculateSales() which uses the sales field internally without calling the getter - it would fail – Veselin Davidov Jul 31 '18 at 13:13
Yes yes. I'll have to call it several times. Absolutely correct. – Prashant Pandey Jul 31 '18 at 13:21
1

Yeah so basically there is no point in delaying it and worrying if your object is in usable state when you can just initialize it in the constructor. That's why you have the constructor otherwise you wouldn't have to make one – Veselin Davidov Jul 31 '18 at 13:23

Calling private methods from inside the constructor in Java

2 Answers2