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?

This is very common in the Active Record pattern.. Nothing wrong with it at all! — Mike Christensen, Jan 23 '12 at 21:22
is there any static state? is it meant to implement any kind of abstraction (interface etc)? — Marc Gravell, Jan 23 '12 at 21:23
@MarcGravell These classes don't contain any static state. At some point they instantiate a wrapper around the underlying database connection, as an aide to constructing the relevant queries. — blueberryfields, Jan 23 '12 at 21:26
@blueberryfields and where do they get the underlying database connection from, if not from static state? Do you pass in the connection as a parameter? — CodesInChaos, Jan 23 '12 at 21:29
I'm still wrapping my head around this code. It looks like the wrapper is instantiated (and creates a DB connection) for each query — blueberryfields, Jan 23 '12 at 21:40

score 5 · Accepted Answer · answered Jan 23 '12 at 21:22

5

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.

answered Jan 23 '12 at 21:22

Darin Dimitrov

1,023,142
271
3,287
2,928

"working with abstractions" is a bit general - what do you mean when you say that? – blueberryfields Jan 23 '12 at 21:24
2

@blueberryfields, concretely abstractions in .NET are either interfaces or abstract classes. So basically your controller action should take an interface or an abstract class as constructor argument which will define the contract of all the operations that you are willing to perform. Then you should configure your favorite Dependency Injection tool to inject a specific implementation of this repository or data access contract into the controller. We are now saying that there is a weak coupling between your controller and the data access layer throughout this contract. – Darin Dimitrov Jan 23 '12 at 21:26

score 2 · Answer 2 · edited May 23 '17 at 12:27

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:

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.
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.

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

2 Answers2