How to make class more testable?

Question

I have this:

 class FooGenerator:IFooGenerator {
      private object _generated;

      public void Generate() {
         // Generating
         GenerateSmallPart();
         GenerateOtherSmallPart();
         GenerateTinyPart();
         // A lot similar
      }

      private void GenerateSmallPart() {
         //add small part to _generated
      }

      private void GenerateOtherSmallPart() {
         //add small part to _generated
      }

      private void GenerateTinyPart() {
         //add small part to _generated
      }
   }

   internal interface IFooGenerator {
      void Generate();
   }

In my application I use only IFooGenerator via IoC but I want to test all those sub methods.

As I found here, one option is to extract class with all sub methods. But why do i need to do that. It is only being used in FooGenerator.

Do you have any suggestions how can i make my class more testable?

As far as I know, you should be testing against public API, not the internals, so you should test against the `IFooGenerator.Generate` and check if the result is what you would expect. But if you really want to test the 'building' part, maybe it's a good idea to extract the `Generate` parts into a builder...? — Patryk Ćwiek, Apr 10 '13 at 08:07
As all those methods don't return anything, I wonder how you want to test them anyway... Are you using Mocks? If so, you can verify them even when you just test the public method. But beware: This most likely creates brittle tests. — Daniel Hilgarth, Apr 10 '13 at 08:07
:) Yeah! start some black box / white box testing flamewar :) For me THE DOCTOR is right. — , Apr 10 '13 at 08:11
@Trustme-I'maDoctor So is it ok to open all these methods to public access even if `FooGenerator` is the only one who will ever use it — Vladimir Nani, Apr 10 '13 at 08:14
@VladimirNani If you *really* want to test the internals then you can make these methods `internal` (semantic satiation starts kicking in...) and then use `InternalsVisibleTo` attribute to expose them to test assembly. — Patryk Ćwiek, Apr 10 '13 at 08:21

score 2 · Answer 1 · answered Apr 10 '13 at 08:29

I will share with you how I usually handle this situation. If I see that I want to test some private methods for some reason (like it is hard to stub input parameters to test all code flows) - this usually means for me that complexity of my class is high and I need to refactor my code. In your case (I have no idea what your methods are doing) you can use something like:

interface IPartGenerator
{
   void GeneratePart();
}

class SmallPartGenerator : IPartGenerator
{
   void GeneratePart();
}

class OtherSmallPartGenerator : IPartGenerator
{
   void GeneratePart();
}

class TinyPartGenerator : IPartGenerator
{
   void GeneratePart();
}

class FooGenerator:IFooGenerator 
{
   private IPartGenerator[] partGenerators = new IPartGenerator[] 
                        {
                           new SmallPartGenerator(), 
                           new OtherSmallPartGenerator(), 
                           new TinyPartGenerator ()
                        }

   public void Generate()
   {
        foreach (var partGenerator in partGenerators)
        {
              partGenerator.GeneratePart();
        }
   }
}

Now you can test each of the part generators separately.

score 1 · Accepted Answer · answered Apr 10 '13 at 08:20

1

Who is the client?

Many people (Roy Osherove, Michael Feathers) consider the test client just as valid as the interface or service client.

With that in mind I think it's fine to slightly go against the principle of encapsulation by opening up testable seams by making some private methods public.

answered Apr 10 '13 at 08:20

Neil Thompson

6,356
2
30
53

Just want to add that if you're ok with going against encapsulation in the name of testability then I recommend you to make some private methods internal and add unit-tests assembly to friends of you prod-assembly with [InternalsVisibleTo attribute](http://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.internalsvisibletoattribute.aspx). – alex.b Apr 10 '13 at 10:50
@aleksey.berezan this InternalsVisibleTo attribute seems for me as a workaround. I really don`t want to go against holy principles :) – Vladimir Nani Apr 10 '13 at 11:22

score 0 · Answer 3 · answered Apr 10 '13 at 10:41

Your class does not one thing but several things - each encapsulated in a private method. This is a violation of the Single Responsibility Principle (SRP). According to SRP, a class should do only one thing.

Neil Thompson suggest that you should make the private methods public. This, at least, make them accessible to unit tests, but they are still SRP violations. If your class has to many different things, initiation is often complex; you have to create the class in a state that satisfies the needs of all methods allthough you only want to test a small corner. This does nothing good for testability.

Following this, outcoldman's answer is a sounder design. His code has no SRP violation.

Does Builder approach that was suggested by Trust me - I'm a Doctor violates SRP? — Vladimir Nani, Apr 10 '13 at 11:24
In my oppiniion, your class still has a lot of different R's (responsibilities) with the kind Doctors approach. SRP means only one R per class, so yes it violates SRP. Once you really dig into unit testing/TDD, you will see how valuable SRP is. — Morten, Apr 10 '13 at 12:04

How to make class more testable?

3 Answers3