I'm working on making my classes unit-testable, using dependency injection. But some of these classes have a lot of clients, and I'm not ready to refactor all of them to start passing in the dependencies yet. So I'm trying to do it gradually; keeping the default dependencies for now, but allowing them to be overridden for testing.

One approach I'm conisdering is just moving all the "new" calls into their own methods, e.g.:

public MyObject createMyObject(args) {
return new MyObject(args);
}

Then in my unit tests, I can just subclass this class, and override the create functions, so they create fake objects instead.

Is this a good approach? Are there any disadvantages?

More generally, is it okay to have hard-coded dependencies, as long as you can replace them for testing? I know the preferred approach is to explicitly require them in the constructor, and I'd like to get there eventually. But I'm wondering if this is a good first step.

One disadvantage that just occurred to me: if you have real subclasses that you need to test, you can't reuse the test subclass you wrote for the parent class. You'll have to create a test subclass for each real subclass, and it will have to override same the create functions.

There are a lot of "new X" scattered throughout the code, as well as many uses of static classes and singletons. So when I try to test one class, I'm really testing a whole bunch of them. If I can isolate the dependencies and replace them with fake objects, I'll have more control over the testing.
–
JW01Jan 29 '11 at 20:29

4 Answers
4

This is a good approach to get you started. Note that the most important is to cover your existing code with unit tests; once you have the tests, you can refactor more freely to improve your design further.

So the initial point is not to make the design elegant and shiny - just to make the code unit testable with the least risky changes. Without unit tests, you have to be extra conservative and cautious to avoid breaking your code. These initial changes may even make the code look more clumsy or ugly in some cases - but if they enable you to write the first unit tests, eventually you will be able to refactor towards the ideal design you have in mind.

Just a remark on this "once you have the tests, you can refactor more freely" - If you don't have tests, you aren't refactoring, you're just changing the code.
–
SlomojoJan 30 '11 at 0:40

@Slomojo I see your point, but you can still do a lot of safe refactoring without tests, particularly automated IDE ractorings like, for example (in eclipse for Java) extracting duplicated code to methods, renaming everything, moving classes and extracting fields, constants etc.
–
AlbJan 30 '11 at 1:48

I'm really just citing the origins of the term Refactoring, which is modifying code into improved patterns which is secured from regression errors by a testing framework. Of course, IDE based refactoring has made it far more trivial to 'refactor' software, but I tend to prefer avoiding self-delusion, if my software has no tests, and I "refactor" I'm just changing the code. Of course, that may not be what I tell someone else ;)
–
SlomojoJan 30 '11 at 1:52

1

@Alb, refactoring without tests is always more risky. Automated refactorings sure decrease that risk, but not to zero. There are always tricky edge cases which tools may not be able to handle correctly. In the better case the result is some error message from the tool, but in the worst case it may silently introduce subtle bugs. So it is advised to keep to the simplest and safest possible changes even with automated tools.
–
Péter TörökJan 30 '11 at 14:43

for all properties instead of just adding @Inject to their definition. This will allow you to treat them as normal beans, which you can new when testing (and setting the properties manually) and @Inject in production.

When I am faced with this type of problem I will identify each dependency of my class-to-test and create a protected constructor that will allow me to pass them in. This is effectively the same as creating a setter for each, but I prefer declaring dependencies in my constructors so that I don't forget to instantiate them in the future.

This will allow you to refactor internally so that you can reference your myObject through the getter. You never have to call new. In the future, when all clients are configured to allow dependency injection, then all you have to do is remove the creation code in one place - the getter. The setter will allow you to inject mock objects as required.

The above code is only an example and it does directly expose internal state. You should carefully consider if this approach is appropriate for your codebase.

Thanks. I'm considering this approach for some cases. But what do you do when MyObject's constructor requires parameters that are only available from inside your class? Or when you need to create more than one MyObject during the lifetime of your class? Do you have to inject a MyObjectFactory?
–
JW01Jan 29 '11 at 20:40

@JW01 You can configure the getter creator code to read internal state from your class and just fit it in. Creating more than one instance during the lifetime is going to be tricky to orchestrate. If you run into that situation, I'd suggest a redesign, rather than a work around. Don't make your getters too complex or they'll become a millstone around your neck. Your objective is to ease the refactoring process. If it seems too difficult, move to a different area to fix it there.
–
Gary RoweJan 29 '11 at 20:47

@DarioOO Actually it's a very poor singleton. A better implementation would use an enum as per Josh Bloch. Singletons are a valid design pattern and have their uses (expensive one-off creation etc).
–
Gary RoweJul 22 '13 at 11:54