Javalobby

I'm an Agile and Lean Strategist specialised in coaching and managing the transformation of IT departments, from startups to enterprise-scale organisations, to highly efficient, productive and energised environments. I also have experience as senior development manager and architecture governance in large enterprises, especially in the finance sector. Marco is a DZone MVB and is not an employee of DZone and has posted 26 posts at DZone. You can read more from them at their website. View Full User Profile

Reflection or the "setter" antipattern?

03.17.2011
| 6127 views |
  • submit to reddit

Recently I came across an interesting dilemma. For some time up to now I've been including setter methods in my service interfaces to allow unit tests to setup mocks. The code though had a bad smell; all too often I kept asking the following question: why did my interfaces need to expose setters for internal details (e.g. the components used to achieve a certain functionality)? If I was a user of this API, would I like to see a bunch of setter methods which have nothing to do with the business functionality? All the time the answer was no.

An API should expose the minimum possible, both in terms of actual classes/operations and in terms of state. A business service is just that:  a collection of operations and state to perform some business functionality: exposing to clients setter methods which allow to setup the internal components is not necessary nor optimal; it just clutters the API with unnecessary signatures. This is what I call the setter antipattern.

So since I'm using Spring test support classes also for my unit testing, I came across the ReflectionTestUtil class, which in few words allows one to do so:

Service service = new ServiceImpl();

Dao dao = new ServiceDao();

ReflectionTestUtil.setField(service, "serviceDao", dao); //serviceDao should actually be a constant

With this class I can avoid exposing unnecessary setter methods in my interface (Service in this case) while still preserving encapsulation. However the price to pay is compile time check. Let's say that a refactoring renames the Service attribute "serviceDao" into something else, say "clientServiceDao" or whatever...Now my test is broken because the ReflectionTestUtil statement will fail.

So which one to chose? The setter antipattern or reflection? I think the answer depends on how much value one gives in keeping a clean API, free from boilerplate signatures. I realised that to me a clean API represents the greatest value, and if someone refactored my code by renaming variables used by the ReflectionTestUtil class, the tests would fail and the fix would be, although annoying, quite easy to make, especially if the field name was captured in a String constant.

 

From http://tedone.typepad.com/blog/2010/10/reflection-or-the-setter-antipattern.html

Published at DZone with permission of Marco Tedone, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Guillaume Jeudy replied on Thu, 2011/03/17 - 9:19am

Unit tests are for testing implementations so why not just keep the setters on the implementation and use the implementation reference in your unit test?

 That's how I solve this problem.

Erwin Mueller replied on Thu, 2011/03/17 - 9:28am

Neither? You should test for functionality and not for internal state. It sounds like you have no idea what your service class is actually for and so you just test for some state.

Either the Service is just to load/save Dao objects, then you obviously have already getter/setter for the dao objects, or your service is doing something else (like a CreditCardService), so you can test for the functionality.

Tomasz Nurkiewicz replied on Thu, 2011/03/17 - 9:31am in response to: Guillaume Jeudy

The problem is more complex in Spring environment, because Spring AOP (used e.g. for declarative transactions) wraps classes in several layers of proxies. This means that you can no longer use concrete implementation as the class implementing your interface is no longer your own service but just a reflective proxy. You'll get something like "Cannot cast $Proxy107 to MyServiceIml".

Xavier Dury replied on Thu, 2011/03/17 - 9:36am in response to: Tomasz Nurkiewicz

but if using spring, I guess you will let it inject the right dependencies...

Angelo Genovese replied on Thu, 2011/03/17 - 9:41am in response to: Erwin Mueller

If your service makes use of DAOs to access persistence, then you wouldn't want your unit tests to test the entire persistence framework, and the DB itself, would you? In your "CreditCardService" example, you would need to record an audit trail of the transactions performed, possibly using a DAO, which you would want to inject as a Mock Object.

Josh Marotti replied on Thu, 2011/03/17 - 10:12am in response to: Tomasz Nurkiewicz

Spring users should use the spring unit test loader and have a test context that injects concrete classes that do the setting for you in configuration.

Josh Marotti replied on Thu, 2011/03/17 - 10:15am

I would argue that if you are testing through an interface, I wouldn't call it true 'unit testing' but, rather, integration testing.  A unit test should test the implementation.

If you are integration testing, set the fields as you would through your normal application (like loading spring and injecting the concrete classes).  True, using spring means you aren't unit testing, either, but, like I said, you are integration testing... there is nothing wrong with integration testing, and I really think they should be used with unit tests, because you can change the underlying implementation and still have a test suite that should still work.

Sebastian Beltramini replied on Thu, 2011/03/17 - 11:42am

First I would say you need to test implementations, not interfaces. So there would be no problem to access to the setter.

Also if you're unit testing, the collaborating class shouldn't be an actual implementation but a mock.

 

Wujek Srujek replied on Thu, 2011/03/17 - 12:14pm

Like others before me - test implementations, not the interface. You are creating the implementatin instance anyways, so don't assign it to the interface, or assing it *after* it is fully initialized (with all setters called).

Personally, I prefer constructor injection, but sometimes it is a pain if there are many deps - but this is also a code smell, which means that your class might be trying to do a little too much.

All interfaces should be mocks generated by a lib (I am using mockito and loving it) or your own test doubles, if it makes things simpler. Only then can you call that a unit test and can you test all you want (behaviour on exceptoins thrown by deps,on invalidinput and so on and the like).

I am glad that you called it an antipattern and that others agree. I have a colleague on the team who read 'Clean Code', thinks only his code is clean, and then again, writes code that makes you puke. I think he didn't understand a word from that book - english is not a virtue of his. Anyways, he puts setters and getters a) for literally everything b) in the interfaces c) and he has interfaces for literally everything! Countless times have I hinted that it doesn't make any sense, only to get scorned. Now I can point him to this post. And the comments are valuable as well. Maybe authority of 'internet programmers' will mean more to him than my remarks (I am the youngest, the least experienced when it comes to years in the business, so not everyone is treating me serious enough ;d). Will test it tomorrow ;d

Matt Avery replied on Thu, 2011/03/17 - 12:20pm

In this case, I would use a technique I call "cutting".  Assuming the unit test is going to be testing the logic on a particular implmentation of Service, in this case ServiceImpl, you can provide a protected "getDao()" method that returns a Dao and use that method internally in ServiceImpl rather than using the field "serviceDao" directly.  In the unit test, you can then "cut in" by creating an in-line subclass of ServiceImpl that returns a mock Dao:

final Dao mockDao = mock( Dao.class );

// mock out Dao behavior here

ServiceImpl service = new ServiceImpl()

{

    protected Dao getDao()

    {

         return mockDao;

    }

};

// make unit test assertions to test all logic within ServiceImp... except of course "getDao()"

No need for a public setter method.  Also, if you change the method name of getDao() or the field name, the unit test will not break since the IDE will rename getDao() for you.

Jakob Jenkov replied on Thu, 2011/03/17 - 12:47pm in response to: Matt Avery

... AKA subclass mocking

Guillaume Jeudy replied on Thu, 2011/03/17 - 1:28pm in response to: Jakob Jenkov

To summarize, both integration testing and unit testing make sense. If you do integration testing you want to use a spring testing context and test all the way down the database. If you do unit testing you want to test the implementation and mock any collaborators using popular mocking libraries.

 The subclass mocking technique is only useful if you are in a legacy application and you don't have the freedom to refactor. In general favor mocking interfaces. Collaborators should always be interfaces that are easy to mock.

Marco Tedone replied on Fri, 2011/03/18 - 5:30pm in response to: Guillaume Jeudy

To me unit tests are to generate the API and object collaboration. I leave functionality test to integration tests. The reason why I don't like using setters in the reference implementation is that these should be public methods and I like programming by interface, not by implementation. Now we would have setters methods for services (e.g. not business related) as part of the public contract although not part of the interface.

Marco Tedone replied on Fri, 2011/03/18 - 5:31pm in response to: Erwin Mueller

Please see my reply on Typepad :-)

Marco Tedone replied on Fri, 2011/03/18 - 5:32pm in response to: Tomasz Nurkiewicz

I didn't think of this aspect. Well said Tomasz

Marco Tedone replied on Fri, 2011/03/18 - 5:37pm in response to: Angelo Genovese

In fact I mock the DAO. That was not the point of my blog. The point was: once I have got an instance of a (mocked) DAO,  how do I set it in the Service? I like using services like Spring ReflectionTestUtils because it allows me not to declare setters in my interface. This of course works only if one uses Spring annotations, because with Spring XML one is forced either to have setters or to declare attributes in the constructor.

Alex(JAlexoid) ... replied on Sat, 2011/03/19 - 10:47am in response to: Marco Tedone

Now we would have setters methods for services (e.g. not business related) as part of the public contract although not part of the interface.

 Now that is contradicting itself. Interfaces are usually considered as a public contract, not implementations. And since the getters and setters are on the implementation, they are part of a private API. And usage of the term API does not imply that it's public in any way.

So you are basically trying to solve a non-issue for most.

Alessandro Santini replied on Sun, 2011/03/20 - 3:58pm in response to: Marco Tedone

I think that once you have created a concrete instance of your service, you can safely exploit a setter to inject the dependency. In the scope of unit testing, this is perfectly acceptable; just to do not let the setter be part of the public (business) service interface and you'll be alright.

Sébastien Dubois replied on Tue, 2011/03/22 - 10:45am

I agree with Alessandro, I don't understand why you would need to touch the public interface of your services in order to be able to unit test the private methods of a specific implementation. If you're unit testing a given implementation, then it's perfectly fine to manipulate the implementation itself (white box testing). Check this page if you need to know more about white box/black box testing: http://stackoverflow.com/questions/402161/black-box-vs-white-box-testing

Xavier Dury replied on Sat, 2011/03/26 - 3:25am in response to: Alex(JAlexoid) Panzin

So you are basically trying to solve a non-issue for most.
+1

Liezel Jane Jandayan replied on Thu, 2011/08/25 - 7:19am

Protocols are usually shared between different technologies system based on given computer programming languages in a given operating system and usually allow the different technologies to exchange information, acting as an abstraction/mediation level between the two. While APIs are specific to a given technology: hence the APIs of a given language cannot be used in other languages, unless the function calls are wrapped with specific adaptation libraries.-Jonathan Berkowitz

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
"Starting from scratch" is seductive but disease ridden
-Pithy Advice for Programmers