• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Dependency injection and mocking with reflection.

 
Saloon Keeper
Posts: 15608
366
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm a huge fan of dependency injection, and Inversion of Control frameworks in particular. I'm very well aware of the fact that using dependency injection makes unit testing a breeze in combination with mocking frameworks.

Recently I've encountered a lot of code containing static helper classes, responsible for returning application state. One such class is a Config class that returns configuration settings from some database. When I voice my concerns over not being able to mock this Config class, I am pointed out that the testing framework we use can mock private and static members of these helper classes. The code is written in C#, here's an example:
This left an extremely bad taste in my mouth, but I can't explain why my concerns are valid. After all, helper classes *are* easy to use and understand, and apparently we can mock them as well with the help of some reflection.

Can anybody explain why this is a bad idea, or whether this spells bad news for inversion of control?
 
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
In his book "Effective Unit Testing":

Lasse Koskela wrote:
Most static methods shouldn’t be. The reason to have them is generally either
because they don’t relate to a specific instance of a class or because we couldn’t bother
figuring out where it belongs so we made it a static method in this utility class. The former
motive is solid but the latter is mere ignorance or lack of interest. Yet another reason,
and an unfortunately common one, is to make it easier to provide global access to
the method by making it static.
Some methods are naturally static. For example, a utility method that performs a
calculation on numbers would likely be a good candidate for a static method. So
what distinguishes a method as one that should or shouldn’t be static? Perhaps the
example in the following listing serves to make the idea more clear.

The rollDie() method produces a random result as if you’d rolled a die with a given
number of sides. Dealing with random factors in automated tests is generally something
you should avoid, so you’ll likely want to stub out the rollDie() method in your tests.
My rule of thumb is to not make it static if you foresee that you might want to
stub it out in a unit test one day. It turns out that I rarely need to stub out a pure calculation.
On the other hand, I frequently find myself wanting to stub out static methods
that serve as an entry point to a service or a collaborator object.
Pay attention to the kind of methods you declare static. A static method is easy to
write, but you’ll have a hard time later if you ever need to stub it out in a test. Instead,
create an object that provides that functionality through an instance method.


So in your example, there is no compelling reason to have a method for interrogating the OS or database as a static method. In fact, the difficulty of testing is a far more compelling reason for them to not be static. As a general rule, if I find myself needing to mock private or static methods then I take that as a warning that my design in wrong.

As a temporary measure when working with Legacy code, as you are, I would wrap the static call with an instance class for easy replacement with a test double. For example:

Using static methods for database calls is an example of tight coupling, which is why it's difficult to test.
 
Stephan van Hulst
Saloon Keeper
Posts: 15608
366
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks for your response Tim.

Tim Cooke wrote:In fact, the difficulty of testing is a far more compelling reason for them to not be static. As a general rule, if I find myself needing to mock private or static methods then I take that as a warning that my design in wrong.



This has always been my viewpoint, but I have a hard time defending it when my colleagues say "we can easily mock static methods". I'm looking for a good reason why using static methods (which traditionally shouldn't be static) is bad, even when it's possible to replace their behavior during testing.

I'm not going to switch to the dark side, but I need some restoration of faith :P
 
Tim Cooke
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Your colleagues are right in that you can mock static calls in your tests, however I would contend that it's 'easy' and it's certainly not nice. Even with the testing tools available for this kind of thing, the resulting test code is normally quite unpleasant.

There a couple of things I'd say about it:

The main downside of your design is one of coupling. Having your database code in a static method is tightly coupling your application to your database implementation. What if in the future you decided to change database vendor or decide not to use a database at all, how easy will that be to do? Not very easy because any change has a direct effect on every client of the class. Polymorphism is a powerful feature of Java which would be useful in this case, but you are not making use of it. Decoupling the implementation with an interface would ease in the transition in this example (and as an added bonus would make it simper to test).

A lot of the testing tools available for mocking static and private method calls do make it 'easy' to do this, if you're comparing it to the alternative of using reflection directly. However, in my experience these tools add a lot of noise to your test cases and have a tendency to hide the real intent of the test making them less readable and thus harder to maintain. I might have a different opinion on this point if there were tools available that provided a nice clean way of stubbing out static calls, but given that they are essentially window dressing for the Reflection library I don't expect this to be a reality any time soon.

In general, using static methods is not an inherently bad thing. Functional languages do it all the time! However, Java is an Object Oriented language for which its core features, constructs, tools, and testing tools are all geared towards dealing with objects. When you move away from using objects and start working with static functions you choice of available tools diminishes significantly and you have to start conjuring magic tricks in your tests to get a handle on your code again.

I have a rule of thumb when it comes to unit testing. I expect the level of complexity of my unit tests to be roughly equal to the complexity of the code under test, and if this is not so then a red flag goes up. This is why I hesitate to use heavy frameworks necessary to mock static and private calls. Should a simple method that marshals the retrieval of some values from an external system require the use of Reflection? I say not. There's a significant imbalance of complexity there, which smells like bad news.

In summary, yes you can mock static calls. But the necessity to do so likely indicates that there's a better design begging to be discovered.
 
Rancher
Posts: 2759
32
Eclipse IDE Spring Tomcat Server
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
They are using a Singleton pattern, and then mocking the Singleton for testing. There are many documented drawbacks to the Singleton pattern. In this case the biggest one is...

They are essentially tying the code that uses one of their classes to the Config singleton. Essentially, if they want to take a class out of Project A and use it in Project B, the Config class will have to go with it the class. This means that essentially Project B needs to have the same properties files as Project A. But, project B has it's own properties files!

The problem is not testing. The problem is that they are violating the Single Responsibility Principle. A class that talks to the database should only talk to the database. It shouldn't be responsible for talking to the config class to read the configuration items. In an IoC container, the container is responsible for reading the configuration files.
 
Bartender
Posts: 689
17
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This problem is exacerbated by some poor design decisions by the language makers (in my case Java but I'm sure the same problem exists in other languages). My application needs to perform basic operations on the filesystem (checking files exist and reading their contents), and java provides the Files class that has a lot of nice functionality for doing that.

Unfortunately the Files class consists entirely of static methods. This means that any component that uses it can then not be Unit Tested since it depends on the file system. The only way to use it and maintain testability is to produce a facade that just forwards all method calls to Files but that provides non-static methods, thus allowing it to be mocked.
 
Stephan van Hulst
Saloon Keeper
Posts: 15608
366
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I tend not to use the File or Files classes that much. Generally when I need to perform I/O, I have my methods accept either an InputStream or a Reader.

If you need to mock methods from the Files class, you can either specify your own default FileSystemProvider (Files delegates to such an instance), or you can pass a FileSystemProvider to whatever class is responsible for performing operations on files, and have it use the provider directly, rather than the Files class.

Honestly, I think it should be illegal for fields to be static if they're not value types.
 
Don't get me started about those stupid light bulbs.
reply
    Bookmark Topic Watch Topic
  • New Topic