1

I have been following some guidelines with Guice and it mentions that you should never use new in constructor or in fields, because than it is inflexible design preventing from proper unit testing.

What does that mean?

And an additional question, if my function reads an XML file and parses it, it is a scope of a unit test to try to catch all possible incorrect input values? In UI, we do not write unit tests for input validation..

GhostCat
  • 137,827
  • 25
  • 176
  • 248
John V
  • 4,855
  • 15
  • 39
  • 63
  • Misko Hevery's article ["Flaw: constructor does real work"](http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/) is a good read on this topic. – Andy Turner Feb 10 '17 at 07:50

1 Answers1

7

Never is too harsh. Very often might be a more appropriate statement.

The point is: in order to do proper unit testing, you sometimes have to control the objects that your "class under test" is using. And when the corresponding field is initialized via a call to new() ... then you simply can't control that field. ( well, you could be doing that using bytecode-manipulating tools like PowerMock; but especially when writing your own code: then you absolutely avoid PowerMock; the need to use that indicates that you put down a bad design. Period )

Whereas, when you use dependency injection, then a unit test can pass a mocked object for that field into the class under test.

But please note: this is not only about "proper unit testing." There is an essential difference between A) initializing your business logic and B) your business logic doing its "business". Those are two different responsibilities, and when you are not clear about this separation, your product is much harder to maintain and enhance over time.

A good starting point to learn about this topic are these videos. Each one really worth each minute ...

EDIT: your comment/question goes quite far; and there is ton of existing material out there (like here, here, or there). But one general remark here: the essence of reasonable unit testing is ... to segregate your business logic into small, testable units!

What I mean is: when you push a lot of code into one class that does 5 different things, then writing good unit tests is hard. Instead: you create one unit for one responsibility. In your context: there could be one class for reading XML data. That class deals with XML, and returns something that is not XML any more. Then you have another class that works on the output created by the first class.

And about your second question: that really depends where your XML is coming from. In other words: your production code must be able to detect all kinds of bad XML input. Because you want your production code to correctly handle all input that could be given to it. So your the goal of your unit tests is to make sure that all validation that you put into place is actually working. It is not the job of your test code to validate XML! Unit tests validate that your code fulfills its contract!

And now, your unit tests focus on that: some tests make sure that this XML reading/transformation works; and other tests (that know nothing about XML) make sure that the further processing works.

But as said; this is really a very broad topic, and you shouldn't expect that a single SO answer can tell you all the things you need to do. This topic requires many months of studying all kinds of resources, and ... practicing, practicing, practicing...

Buttle Butkus
  • 9,206
  • 13
  • 79
  • 120
GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • Just to add, with TDD, this comes naturally. – GauravJ Feb 10 '17 at 07:21
  • 2
    Yes; and no. Of course, TDD helps writing production code that is easier to test. But the core point is: even when you do TDD; it can be perfectly fine to have a field that does a new call directly. Example: your production code needs some stack to store data. I think it is perfectly to do that "in place" and not have DI for that - because that is kind of the "inner logic" of your class; and the outside world doesn't need to know about that. – GhostCat Feb 10 '17 at 07:25
  • Thanks a lot. If you can comment on the other point - is in the actual scope of unit testing checking for incorrect inputs, such as in XML loading? That would be many cases then – John V Feb 10 '17 at 07:31
  • Well with DI as well , you would create Stack, Maps and List using new operator. It's harder to explain that DI is not just passing objects in constructor (it's a mean to do DI) but to invert flow (like framework). If the objective is to write a unit test for a class especially for beginners TDD, it forces you make dependencies more explicit. – GauravJ Feb 10 '17 at 07:33
  • To add a bit more on what has been already mentioned, it is also very much easier if you use a DI framework. For instance Spring, or any other tool, that gives also the possibility to inject your components with an annotation (Autowired in the case of Spring) and promotes the practice of composition. Composition is key. It means that you delegate to other class when your class starts doing too many things, so that other class is tested in isolation with the help of mocked objects. It is true that using TDD all this come naturally. Always promote composition, and always promote Unit Testing. – Fernando Feb 10 '17 at 07:37
  • Thanks, I just got a bit confused by your statement: "the goal of your unit tests is to make sure that all validation that you put into place is actually working. It is not the job of your test code to validate XML! " So should I write a unit test for each possible invalid input? – John V Feb 10 '17 at 08:21
  • @user970696 You should write a unit test that makes sure that each different kind of validation that your production code checks for ... is handled correctly. The point is: the correct answer very much depends on your specific requirements; that ... simply too much content for this place. My recommendation: think about potential inputs, write some code, and see where it gets you. You could then put some example code to codereview.stackexchange.com and ask folks have a look into it. – GhostCat Feb 10 '17 at 08:24
  • Also when I want to make sure null input is correctly handled, I write a unit test to cover that (so pasing null and checking that correct error is shown, for example). – John V Feb 10 '17 at 08:26
  • @user970696 Exactly: you want to have at least one unit test for each different behavior that your production code can be showing. – GhostCat Feb 10 '17 at 08:31