Using static methods to interface with a database - any potential problems?

Question

I'm looking at a class handling database access for an MVC3/.Net application.

The class is static, and provides nice convenience methods for common DB queries - all sorts of twiddly stuff like "GetColumnBValueForColumnA()", as well as much more complex queries. It's nicely factored/optimized for the given solution and domain.

Seeing the class as static, though, triggered some half forgotten memory about how this might be a bad idea (maybe in the context of multi-threading?), and I can't shake the feeling.

Is it a good idea to keep this kind of class static, or should I be instantiating it for each database call?

Answer 1

Is it a good idea to keep this kind of class static, or should I be instantiating it for each database call?

If you care about things like weak coupling between the layers of your application, reusability of those layers, unit testing in isolation you shouldn't be doing any of the above. You should be working with abstractions.

If you don't care about those things then static methods are just fine. The only thing to be careful when working with static methods is to design them so that they are reentrant and do not depend on any shared state in order to be thread safe. In all cases make sure to properly dispose all IDisposable resources such as database connections and commands by wrapping them in using statements.

Answer 2

Is it a good idea to keep this kind of class static, or should I be instantiating it for each database call?

These are not your only two options.

The class should not be static: by making it static you relinquish a few important advantages of object-oriented programming, while gaining practically nothing.

Instead, an instance of it should be provided to your controllers via constructor-based dependency injection. Whether the class is re-instantiated for each request or whether you end up using a singleton is then determined by your DI-binding code. You can change it at the drop of a hat.

Here are a couple of advantages:

1. Say you want to unit test the method that relies on this data-access class. If you are using an injected service interface rather than calling a static method directly, you can create a Mock object, tell it how to behave, and allow the method you're unit testing to think it's getting real data when in fact you're just feeding it the data you want to test.
2. Say you end up wanting to cache the results of your data-access calls. You could inject a caching class that wraps the actual class, returning cached results where possible and invoking the real data-access methods when cached values aren't present. None of this would require any changes to your data-access class.

I could go on. Static classes kill your flexibility and have no practical advantage 99% of the time.