Wednesday, February 23, 2011

Remember to give a hint

We ran into once interesting discussion during our last code review session. As we all do it's a common trick to use relax access restrictions to test a particular property/method/function, so that you can use it in a unit test, which resides in the same package (though in different catalog).

Whether you think it's good or bad, I believe always remember to give a hint about that to the developer. A simple annotation that tells you why a particular property access restriction has been relaxed.

Consider:

public class foo {
private Long x;
String y;
}

Hummm Why is 'y' package scoped?

public class foo {
private Long x;
@VisibleForTesting String y;
}

Ah, that's why

Nice isn't it.
Manisha

3 comments:

  1. Nice thought. I see the value this may add when we need to test a private method. However, would'nt we want to use the getter / setter method in unit tests to access a property rather than relax its scope?

    Good counter read on this would be Dane King's blog. http://daneking.blogspot.com/2009/07/keep-your-privates-private.html

    Just my 2 cents !

    ReplyDelete
  2. I agree with you Ayan. My main focus was method but I picked a wrong example probably :). But thanks for sharing your thought.

    ReplyDelete
  3. Nice post Manisha!
    I will assume greenfield in voicing my opinion.
    If a method makes sense to be public, I'd just make it a public regardless of whether it is invoked or not. This way you can easily test it, yet make it available and visible for any possible future needs. Now again, stressing on the assumption of greenfield and also assuming we are doing a true greenfield, which would be to write test first and than actual code, I'd think that tests for the public methods should be robust enough to cover any testing needed for the privates as well...I believe I just wrote the longest sentence in the history of blogging. Ok, just one more thing, legacy speaking, at work we do have an utility to test private methods...which again is a neat thing to have, but yet it should only be used in testing an isolated changes in spaghetti legacy code...
    Just my two rubles!

    ReplyDelete