Hacker News new | ask | show | jobs
by lkitching 3153 days ago
I don't see why TDD requires ruling out static methods and insisting on hiding everything behind an interface. Static methods are straightforward to test, certainly more than a class with multiple dependencies which need to be mocked. Usually the complaint is about coupling when calling static methods but these can be wrapped in a delegate if required.
2 comments

Simply because you can't mock the static dependency, therefore that method is now dependent on the static class and you don't have any control over it. This is problematic - what if at some point later another developer adds a database call into the static method to do some logging? Now your testing will dirty whatever database you're using, as well as run 10x slower - and yet the test will still pass and everyone will be none the wiser as to what happened.

If you start using a custom delegate solution, then your code is not consistent with everything else that uses DI, making it harder to understand. I can understand interfaces are annoying when navigating code, but the IDE still helps with that even if it is a few more button clicks, and the pros outweigh the cons.

> that method is now dependent on the static class and you don't have any control over it.

I don't see how you have any less control over it than any other code you wrote. If you don't want it to write log statements, then don't do that. Most static methods are small and pure so don't need to write log statements anyway.

> Now your testing will dirty whatever database you're using, as well as run 10x slower.

I've never used a logging framework that didn't allow you to configure where log statements were written, or give you control over the logging threshold for individual classes. However if your method is writing logs then presumably there is a reason, which is just as useful in the tests. If you mock it out then you're testing against different code to the one you will actually run against.

> If you start using a custom delegate solution, then your code is not consistent with everything else that uses DI.

Passing functions as arguments directly is 'DI', just without the need to configure that through an external container. Reducing the amount of interfaces (often with a single implementation) and external configuration makes navigating the code easier.

I think you missed my point, it's not about the logging framework, its about the fact you don't control an external dependency during testing. Unit tests are meant to be reproducible, meaning they are done under controlled conditions.

> Most static methods are small and pure

This is very assuming, tests are a way of being specific about your intent.

> its about the fact you don't control an external dependency during testing

If your code is structured using small static functions, you don't have any dependencies in the first place, just arguments you are passed and transform. You will probably create interfaces for external services you depend on, but you can avoid needing to mock them if you express the transform directly.

> This is very assuming

I'm not assuming anything, since I wrote the static method and I also decided to call it, presumably for the result it calculates. Your argument appears to be that static methods could contain bad code but that applies to all code you depend on.

You mean that the tests will depend on the thing being tested? What a crime!

> what if at some point later another developer adds a database call into the static method to do some logging?

Then you have a developer that does not grasp the idea of functions, and how they can help you improve your code. That's a call for education, not for changing your tests.

The point is that tests are omnipresent, people aren't. I've worked at places where all sorts of dumb code has got through because there is no automation in place to stop it, and everyone else is too busy to do code reviews.
In Java, you can use PowerMock to mock or spy anything, even private static final things. I consider it a smell (though excessive mocking even without powermock is its own smell), but it's immensely valuable to get code you can't change (or fear changing because of its complexity and lack of tests, or simply don't have time to change because the refactoring would take a whole sprint) to have some tests.

You don't need interfaces for everything in order to do DI. Interfaces should be used only for having multiple implementations or to break dependency loops.

Other than that I'm in agreement, static methods generally aren't a good idea. They can all too easily balloon into big chunks of imperative code with inner dependencies (static or not) at 10 indentation levels deep. Non static methods can too, but not as easily, and you have more options for fixes/workarounds in those cases anyway. The only place they really make sense is as part of a set of pure primitive data transforms, and ought to be small.

In C# we have Moq that can mock normal classes, though it requires adding 'virtual' to every method you want to override which is a code smell too. In Java everything being virtual by default I guess it doesn't matter. We like to always keep an interface around as it gets the developer used to working that way and keeps the code consistent. Visual Studio provides a quick shortcut to auto gen the interface too.
> Now your testing will dirty whatever database you're using, as well as run 10x slower

It sounds like a problem is a few layers higher. Why is there a live database in your unit testing environment? Why are working credentials configured? If they're unit test, not integration tests, all db operations should be DId / mocked / whatever. Any call that isn't should fail, not take longer time. Db interaction is for the integration tests.

That's exactly my point, mock your external dependencies. Static calls don't allow you to do that.
In your language of choice:

    static_function(db, other, arguments) { ... }
    test { static_function(fake_db, 1, 2) }
You can even omit the db in the standard case if your language allows default keyword arguments. In almost every language, a method is just a fancy static call that takes extra arguments implicitly. (Closures are poor man's objects, objects are poor man's closures...)
So how exactly do you test that your SQL query does the right thing? That you're using the Twitter API correctly?
Testing a database, or an external web service, is an integration test. They can be as simple as:

    void TestCreateUser() {
        var repo = new UsersRepository();
        var mockUser = new User("John", "Smith");
        repo.AddUser(mockUser); // db call
        var addedUser = repo.GetUsers().Single(); // db call
        Assert.StructureIsEqual(mockUser, addedUser);
    }
For the Twitter web service, you might test that you successfully get a response, as you don't have control of what exactly comes back.
How is static code different from other noninjected code, like stuff in a method. Taken to the logical conclusion we'll have thousands or classes full of max 2 operations per method.
How many static classes are your methods using? And what is the problem with injecting this stuff at the top of the class instead? If you plan to write tests, you have to control your dependencies, and DI is the simpliest way to do that.
Because these tests become too detached from reality if you inject everything. A silly example to make the point:

IAdder { double Add(double a, double b) }

Test:

car mockAdder = ... // Mock 1+1=2 etc.

um... yes... this is actually what "pure" OO involves. The only reason we don't do this is because it's a nightmare to manage.
“What if ...” doesn’t pass YAGNI.
Problem is that the moment you start introducing delegates and crap like that is you're inventing a mechanism to work around your resistance to not using static methods rather than actually solving any problems.

There is no functional difference between a class with static methods and a class without, of which one instance is available to other classes.

Other than the fact that it isolates state, allows mocking and substitution and testing.

I disagree that delegates and higher-order function are 'crap' or in any way more complicated than introducing interfaces that are injected though a centralised container. You could just as easily turn that argument around and say mocking and an overuse of interfaces come from your resistance to using small static methods. In C# Linq is almost entirely based on static methods and delegates and that is not harder to test as a result.

Static methods usually don't rely on any hidden state at all. The example originally given was for a graph operation which could just take the input graph as an argument and return the result. When your code is composed of small independent functions you don't need mocking and substitution at all. In my experience most uses of mocks come from functions that do too much in the first place.