loeschg loeschg - 1 month ago 8
Android Question

Is it bad practice to make a method package or private for the sake of testing?

In Java (in an Android specific context, but this should apply across the board), is it considered bad practice to remove a

private
modifier - and thus be
package
specific - for the sake of unit testing?

Say I have something like the following:

public void init(long id) {
mId = id;
loadItems(1);
}

public void refresh() {
loadItems(mId);
}

private void loadItems(int page) {
// ... do stuff
}


In this case, I have 2 public methods that should definitely be tested. The concern is that both the
refresh()
and
init()
methods are almost identical minus some logic around handling the id.

It seems like it'd be easiest to write a unit test for
loadItems()
and then just verify that both
init()
and
refresh()
call
loadItems()
with the appropriate id (using something like Mockito). There's not a "good way" to test private methods though.

Would doing that make me a bad software developer? I know private methods technically shouldn't need unit tests, but this would be a straightforward testing approach, IMO, especially if
loadItems()
is somewhat complex.

kha kha
Answer Source

You aksed "Would doing that make me a bad software developer?".

I don't think it makes you a bad developer. If you look at .NET for example, they even have a way to allow other libraries to see the internals of another library for the purpose of unit testing (InternalsVisibleTo).

I am personally against testing private methods though. In my opinion unit testing should be done on visible methods and not private methods. Testing private methods kind of defeats the point of encapsulation and making a method more visible than it needs to be just for the sake of unit testing looks wrong to me.

If I were you, I would instead test both my public methods. Today, your two methods are almost identical and it's easier to test your private method by making it package visible. Tomorrow however, that may no longer be the case. As both methods are public and easily accessible to other classes, you may be testing the wrong thing if such a scenario happens and the two drift apart.

Better yet (and this is what I would recommend) is to move

private void loadItems(int page) {
    // ... do stuff
}

to its own class with its own interface and then test loadItems(int page) once using a separate unit test and then test the two public methods by just making sure they call the interface with the arguments you're expecting. This way, you're testing your entire code and avoid the pitfall I explained above.