12

So I came across this issues today and I couldn't find some meaningful explanation is there some non-subjective reason to use static methods when it comes to database interactions.

Right now I'm working on a project where everything is made through stored procedures and for example I have some regular methods like :

public void Delete(int clientID)
    {
      //calling store procedure
    }

or

public void Save(int clientID)
    {
      //calling store procedure
    }

but I also have :

public static Client GetByID(int id)
    {
        //calling stored procedure
    }

or

public static int AddNew(string firstName, string lastName...)
    {
      //calling store procedure
    }

and since I'm working with .NET for about 9 months and I've been using only Entity Framework and Repository Pattern I can't recall anywhere or any code where static methods were used. Not for standard CRUD operations, neither for more specific tasks related to the database.

So is this something related to the particular way that the database is accessed, is it some practice that can give (even a very small) performance boost, or it's just the developers approach and I shouldn't give it much of a thought when and when to not use static methods in my database related methods?

Leron
  • 9,546
  • 35
  • 156
  • 257
  • @Anon Yes, it is ASP.NET Web Forms application – Leron Jan 28 '14 at 18:27
  • More reading: http://stackoverflow.com/questions/241339/when-to-use-static-classes-in-c-sharp http://stackoverflow.com/questions/4963920/appropriate-use-of-static-method – Andrew Morton Jan 28 '14 at 18:37

4 Answers4

13

In the particular case of a data access layer I'd avoid static methods for one simple reason... coupling.

Static methods can't implement an interface, instance methods can. By using static methods one is essentially insisting against coding to an interface and instead coding to an implementation. Thus, everything which uses this data access layer is required to this this specific data access layer at all times.

No alternate implementations, no test stubs, no dependency inversion at all. The business logic has a dependency arrow pointing to an infrastructure concern (the data access layer), whereas that should be the other way around.

Additionally, it seems like this at least carries a greater risk of having problems with the disposal of resources. That might not be the case here, but it's really easy for it to become the case. What if a developer somewhere down the road has the bright idea to extract common lines of code into a class-level static method and property? Something like the Connection or DBContext object? That'll create some very interesting and very difficult-to-debug run-time errors.

On the other hand, if repositories were instances then they can simply implement IDisposable and make sure any class-level objects are correctly disposed.

Continuing (I guess I had more objections to the design than I thought), this feels very counter-intuitive to me from an object-oriented sense. Perhaps this one is just personal preference, but this is turning what would otherwise be a "repository object" into a "dumping ground of DB helper methods."

In a system like this I would expect the number of random one-off methods to grow significantly over time as developers make quick solutions to meet requirements without thinking about the overall architecture. Instead of a consistent and well-managed series of objects, you could very likely end up with a bloated and difficult-to-follow codebase.

David
  • 208,112
  • 36
  • 198
  • 279
  • Lol, as if you've described this exact project. I'm impressed how easy and even unnoticeable one thing can lead to another and end up, as you wrote, in a `"bloated and difficult-to-follow codebase"`. I'm gonna read your answer several times, it's a really good one. Thanks. – Leron Jan 28 '14 at 19:18
  • Awesome answer, and pretty perfectly describes my exact pains when trying to add in unit testing to an existing web app project I took over. I will definitely make use of this answer going forward! – wjhguitarman Feb 22 '16 at 14:52
3

Static methods are used when there is either no state, or shared state. If you have code that is merely calling stored procedures, then it may have no state other than a shared database connection string. This would be a valid use of static methods.

Moby Disk
  • 3,761
  • 1
  • 19
  • 38
  • Well, for sure, there's no problem those methods to be static, but yet I wonder is there some more significant reason than the ability to make it static. It's working fine and all, but from what I've seen it's the first time I see someone making this kind of methods static, which from my experience is not the common practice. But of course I might be wrong as well, so I ask.. – Leron Jan 28 '14 at 18:33
  • +1 for mentioning that calling stored procedures could result in the only shared data being the connection string. We use this in our software to maintain data consistency and security. – Michael J. Gray Jan 28 '14 at 18:45
1

I think your last statement is correct; its just the previous developers approach.

I will say, though, that having those methods static is going to make your life a nightmare to create a testable product; if you have the power to change them to be instance based and create a set of unit tests that can test the app via mocks without needing a database it'll make your life easier in the long run.

Allan Elder
  • 4,052
  • 17
  • 19
  • 1
    We can't be sure from the posted example, but if these methods are merely calling stored procedures, then there may be nothing here to test or nothing to mock. – Moby Disk Jan 28 '14 at 18:29
0

We very rarely use them here in our company, although they can serve a purpose in utility classes or things of the sort that are simply calling functions. Depending on how many instances you would expect of a non-static class, they can also affect performance., but that would need to be a notable instance difference.

source - MSDN

K.DW
  • 115
  • 1
  • 8