Unit-testing private methods: Facade pattern

Question

Lots of developers think that testing private methods is a bad idea. However, all examples I've found were based on the idea that private methods are private because calling them could break internal object's state. But that's not only reason to hide methods.

Let's consider Facade pattern. My class users need the 2 public methods. They would be too large. In my example, they need to load some complex structure from the database's BLOB, parse it, fill some temporary COM objects, run user's macro to validate and modify these objects, and serialize modified objects to XML. Quite large functionality for the single metod :-) Most of these actions are required for both public methods. So, I've created about 10 private methods, and 2 public methods do call them. Actually, my private methods should not necessarily be private; they'll not break the internal state of instance. But, when I don't wont to test private methods, I have the following problems:

1. Publishing them means complexity for users (they have a choice they don't need)
2. I cannot imagine TDD style for such a large public methods, when you're to write 500+ lines of code just to return something (even not real result).
3. Data for these methods is retrieved from database, and testing DB-related functionality is much more difficult.

When I'm testing private methods:

1. I don't publish details that would confuse users. Public interface includes 2 methods.
2. I can work in TDD style (write small methods step-by-step).
3. I can cover most of class's functionality using test data, without database connection.

Could somebody describe, what am I doing wrong? What design should I use to obtain the same bonuses and do not test private methods?

UPDATE: It seems to me I've extracted everything I was able to another classes. So, I cannot imagine what could I extract additionally. Loading from database is performed by ORM layer, parsing stream, serializing to XML, running macro - everything is done by standalone classes. This class contains quite complex data structure, routines to search and conversion, and calls for all mentioned utilities. So, I don't think something else could be extracted; otherwise, its responsibility (knowledge about the data structure) would be divided between classes.

So, the best method to solve I see now is dividing into 2 objects (Facade itself and real object, with private methods become public) and move real object to somewhere nobody would try to find it. In my case (Delphi) it would be a standalone unit, in other languages it could be a separate name space. Other similar option is 2 interfaces, thanks for idea.

Answer 1

I think you are putting too many responsibilities (implementations) into the facade. I would normally consider this to be a front-end for actual implementations that are in other classes.

So the private methods in your facade are likely to be public methods in one or more other classes. Then you can test them there.

Answer 2

Private methods are used to encapsulate some behavior that has no meaning outside of the class you are trying to test. You should never have to test private methods because only the public or protected methods of the same class will ever call private methods.

It may just be that your class is very complex and it will take significant effort to test it. However, I would suggest you look for abstractions that you can break out into their own classes. These classes will have a smaller scope of items and complexity to test.

Answer 3

I am not familiar with your requirements & design but it seems that your design is procedural rather than object oriented. i.e. you have 2 public methods and many private methods. If you break your class to objects where every object has its role it would be easier to test each of the "small" classes. In addition you can set the "helpers" objects access level to package (the default in Java, I know there is a similar access level in C#) this way you are not exposing them in the API but you can unit test them independently (as they are units).

Answer 4

Could somebody describe, what am I doing wrong?

Maybe nothing?

If I want to test a method I make it default (package) scope and test it.

You already mentioned another good solution: create an interface with your two methods. You clients access those two methods and the visibility of the other methods don't matter.

Answer 5

Maybe if you take time and look the Clean Code Tech talks from Miško. He is very insightfull of how code should be written in order to be tested.

Answer 6

This is a bit controversial topic... Most TDDers hold opinion that refactoring your methods for easier unit testing actually makes your design better. I think that this is often true, but specific case of private methods for public APIs is definitely an exception. So, yes, you should test private method, and no, you shouldn't make it public.

If you're working in Java, here's a utility method I wrote that will help you test static private methods in a class:

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import junit.framework.Assert;

public static Object invokeStaticPrivateMethod(Class<?> clazz, String methodName, Object... params) {
    Assert.assertNotNull(clazz);
    Assert.assertNotNull(methodName);
    Assert.assertNotNull(params);

    // find requested method
    final Method methods[] = clazz.getDeclaredMethods();
    for (int i = 0; i < methods.length; ++i) {
        if (methodName.equals(methods[i].getName())) {
            try {
                // this line makes testing private methods possible :)
                methods[i].setAccessible(true);
                return methods[i].invoke(clazz, params);
            } catch (IllegalArgumentException ex) {
                // maybe method is overloaded - try finding another method with the same name
                continue;
            } catch (IllegalAccessException ex) {
                Assert.fail("IllegalAccessException accessing method '" + methodName + "'");
            } catch (InvocationTargetException ex) {
                // this makes finding out where test failed a bit easier by
                // purging unnecessary stack trace
                if (ex.getCause() instanceof RuntimeException) {
                    throw (RuntimeException) ex.getCause();
                } else {
                    throw new InvocationException(ex.getCause());
                }
            }
        }
    }

    Assert.fail("method '" + methodName + "' not found");
    return null;
}

This could probably be rewritten for non-static methods as well, but those pesky private methods usually are private so I never needed that. :)

Answer 7

suppose you have 8 private methods and 2 public ones. If you can execute a private method independently, i.e. without calling any of the other methods, and without state-corrupting side-effects, then unit testing just that method makes sense. But then in that case there is no need for the method to be private!

in C# i would make such methods protected instead of private, and expose them as public in a subclass for testing

given your scenario, it might make more sense for the testable methods to be public and let the user have a true facade with only the 2 public methods that they need for their interface

Answer 8

Here is blog on testing private method in C#

http://www.coolaspdotnetcode.com/Web/NunitTesting-testingprivatemethod.aspx