Testing Practices – Is It Bad Practice to Modify Code Strictly for Testing?

cprogramming practicesrefactoringtestingunit testing

I have a debate with a programmer colleague about whether it is a good or bad practice to modify a working piece of code only to make it testable (via unit tests for example).

My opinion is that it is OK, within the limits of maintaining good object oriented and software engineering practices of course (not "making everything public", etc).

My colleague's opinion is that modifying code (which works) only for testing purposes is wrong.

Just a simple example, think of this piece of code that is used by some component (written in C#):

public void DoSomethingOnAllTypes()
{
    var types = Assembly.GetExecutingAssembly().GetTypes();

    foreach (var currentType in types)
    {
        // do something with this type (e.g: read it's attributes, process, etc).
    }
}

I have suggested that this code can be modified to call out another method that will do the actual work:

public void DoSomething(Assembly asm)
{
    // not relying on Assembly.GetExecutingAssembly() anymore...
}

This method takes in an Assembly object to work on, making it possible to pass your own Assembly to do the testing on. My colleague didn't think this is a good practice.

What is considered a good and common practice ?

Best Answer

Modifying code to make it more testable has benefits beyond testability. In general, code that is more testable

Is easier to maintain,
Is easier to reason about,
Is more loosely coupled, and
Has a better overall design, architecturally.

Related Solutions

Testable Code – Is It Better Code?

In regard to the common definition of unit tests, I'd say no. I've seen simple code made convoluted because of the need to twist it to suit the testing framework (eg. interfaces and IoC everywhere making things difficult to follow through layers of interface calls and data that should be obvious passed in by magic). Given the choice between code that is easy to understand or code that is easy to unit test, I go with the maintainable code every time.

This doesn't mean not to test, but to fit the tools to suit you, not the other way round. There are other ways to test (but difficult-to-understand code is always bad code). For example, you can create unit tests that are less granular (eg. Martin Fowler's attitude that a unit is generally a class, not a method), or you can hit your program with automated integration tests instead. Such may not be as pretty as your testing framework lights up with green ticks, but we're after tested code, not the gamification of the process, right?

You can make your code easy to maintain and still be good for unit tests by defining good interfaces between them and then writing tests that exercise the public interface of the component; or you could get a better test framework (one that replaces functions at runtime to mock them, rather than requiring the code to be compiled with mocks in place). A better unit test framework lets you replace the system GetCurrentTime() functionality with your own, at runtime, so you don't need to introduce artificial wrappers to this just to suit the test tool.

PHP Unit Testing – Method Per Test vs Data Provider

It depends.

You should ask yourself: what property of the code is this particular test testing? What do I know when I know that this test is green?

I don't mean this as a philosophical exercise, but in a very practical sense. Take your Adder, for example. You can ask it "Can you add 2 and 2 correctly?" and "Can you add 2 and 3 correctly?" So you would write these tests:

public function knowsTwoPlusTwoIsFour ()
public function knowsTwoPlusThreeIsFive ()

But maybe you don't want to know that. Maybe what you really want to know is "Can you add two ordinay numbers correctly?" (I'll explain what "ordinary" means later) Now your test should use a data provider. The test would be

public function canAddOrdinaryNumbers()

and your data source should include the usual "normal" cases, i.e. adding zero, adding negative numbers, adding its complement:

[4, 2, 2],    // Duh
[-1, 2, -3],  // negative summand
[0, 0, 0],    // just to make sure
[2, 2, 0],    // neutral element
[0, 2, -2],   // inverse

Now your test tells you that Adder has no problems with normal numbers. That's basically one piece of information, hence it should be one test method.

What about numbers that are not ordinary?

public function canAddOneToNaN ()
public function knowsWhatInfPlusNegInfIs ()
public function croaksOnFLoatingPointInput ()
public function whatIsOnePlusSqrtOfMinusOneAnyway ()

I'll leave the implementation of these test cases as an exercise to the reader, and would like to focus on this point: All unit test methods in a class should convey about the same amount of information. That is a subjective measure, of course, but I think it's a good rule: look at the tests and ask yourself: are some of these tests only telling me stuff others already did before? Do some tests seem to have more meaning than others? If so, then you might want to do some refactoring.

Best Answer

Related Solutions

Testable Code – Is It Better Code?

PHP Unit Testing – Method Per Test vs Data Provider

Related Topic