6

I have below constructor, where it creates a workbook in constructor. I read that, ideally, we should not create objects in Constructor, instead, we should have just assignments which are passed.

public ExcelWriter() {
        workbook = new HSSFWorkbook();
        //other code
    }

Is it okay to create fixed objects like above? What is ideal alternative solution? and from Unit Testing perspective? If to pass workbook from calling method code in other class, we have to create the workbook object even there also.

Is later is better approach? How it is better or matters compared to constructor approach?

11

For beginning programmers this is fine.

However, like you said, if you want to unit test this ExcelWriter class, you could perhaps give the HSSFWorkbook object as an argument in the constructor rather than creating it yourself. This way, you can use the real one in your code, and use a mock in the unit tests. This practice is called dependency injection.

So your constructor would look like:

public ExcelWriter(HSSFWorkbook hssfWorkBook) {
    workbook = hssfWorkBook;
    // other code
}

Then, somewhere else in your code, you call that constructor:

HSSFWorkbook myWorkbook = new HSSFWorkbook();
ExcelWriter myExcelWriter = new ExcelWriter(myWorkbook);

And your unit test would be something like:

HSSFWorkbook myMockedWorkbook = // create a mock
ExcelWriter testWriter = new ExcelWriter(myMockedWorkbook);

If you want to do this even better, use interfaces, so you'll end up with two separate classes:

public interface IWorkbook {}

public class RealWorkbook implements IWorkbook {
    // this is the one for in your code
}

public class FakeWorkbook implements IWorkbook {
    // this is the one you use in your unit test
}
  • Just assuming we are talking about the apache POI implementation here - in that case the interface already exists and is called Workbook. Of course, it might still be worth writing some kind of wrapper to make switching libraries at a later point less painful. – Hulk Jan 25 '17 at 10:32
  • I don't know, I've never heard of this Workbook class. Nevertheless I think it's still worth noting about the interfaces. – Jelle Jan 25 '17 at 10:33
5

Nothing in the real world is ideal. But an alternate solution is called dependency injection:

public class ExcelWriter {
    Workbook workbook;
    public ExcelWriter(Workbook workbook) {
        this.workbook = workbook;
    }
    //other code
}

public void main(){        
    Writer writer = new ExcelWriter( new HSSFWorkbook() );       

    //other code
}

This way ExcelWriter can work with any kind of Workbook.

1

It breaks down to semantics.

If the instantiation is in the context of a wrapper to enhance functionality to a Workbook (Inheritance or Composition) then instantiating the Object within the constructor is fine. You would apply the rule "composition over inheritance".

If you compose Objects to create a disjunct new semantic then you should ask yourself if you want to be dependent on the concrete class as you have to know it to instantiate it.

If you have no problem with the concrete dependency (which may violate the dependency inversion principle) object instantiation in the constructor or lazy initialization are semantically identical. Lazy initialization only defers the instantiation until it is neccessary. You abstract from the point of time when an object is created.

If you may have multiple versions of a workbook under an abstraction you should prefer to pass the object in the constructor under this abstraction. On a high level object you should consider container functionality. There several possibilities of dependency injection technologies.

-2

Use lazy initialization. Only create the workbook when someone needs it, e.g. the DoSomething method in this example:

public class ExcelWriter {
    Workbook workbook = null;
    public ExcelWriter() {
        //nop
    }

    protected Workbook getWorkbook() {
        if (workbook == null) workbook = new Workbook();
        return workbook;
    }

    public doSomething() {
        Workbook wb = getWorkbook();
    }
}
  • 1
    While there are places where lazy initialization is indeed what you want, this doesn't solve the dependency problem, creates overhead on each doSomething call and introduces a race condition. – 5gon12eder Feb 4 '17 at 8:11
  • 2
    No, if you need to unit test doSomething you would mock getWorkbook anyway. "Problem" solved. The point of the lazy initialization is to get the logic out of the ctor where it can't be mocked. I understand that DI is the fad these days, but the idea that a class named ExcelWriter shouldn't have a dependency on Excel is missing the point of the class (which is to insulate the main program from the details of working with Excel). As for race condition, maybe one of us is confused, as my code example is single-threaded. – John Wu Feb 4 '17 at 8:22
  • Lazy initialization doesn't only defer expensive operations. It also abstracts from the point of time WHEN an object is created. Introducing concurrency: a potential race condition is not an argument per se. You ALWAYS have to define if objects work under concurrency or not. And if this object is used under concurrency I do not see under all circumstances this class responsible to handle proper concurrency behaviour. – oopexpert Feb 4 '17 at 11:33
  • Regarding DI: DI is a mechanism that is overused in ourdays. Used in a context where you surely have complex behavior encapsulated it is overall beneficial. Providing PersistenceContexts, ThreadPools or access to foreign systems are beneficial usages. Breaking it down to the domain model it becomes an overhead an often an alibi for applying Inversion of Control as DI doesn't fully adresses this issue. It take the part that you do not have to know HOW an object is instantiated and maybe not WHEN it is instantiated. Hiding objects behind interfaces can be done otherwise. – oopexpert Feb 4 '17 at 11:41
  • Yes, this answer does not adress the dependency issue. But the question was not how dependencies are established. The question was when do I know that instantiating objects within the constructor is ok. @5gon12eder The concern you point out would be correct if the question was different. – oopexpert Feb 4 '17 at 11:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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