Wrapping up static class with non-static class

Question

I was looking at a project and I found something very curious.

There is a static class that has a set of methods, where each method makes a call to a remote server.

The template looks kind of like this:

public static class DAI

    public static ResponseObject ExecStatement(string db, string sql)
    {
         { ... }
    }

    public static DataSetResponseObject GetDataSet(string db, string sql)
    {
         { ... }
    }

    public static DataTableResponseObject GetDataTable(string db, string sql)
    {
         { ... }
    }
}

But no where in the project makes a call to this class. Instead, it makes a call to an non-static class container.

public class ExecSP : IExecSP
{
    string _db;
    public ExecSP(string db)
    {
        _db = db;
    }

    public ResponseObject OutputAsResponseObject(string sql)
    {
         return DAI.ExecStatement(_db, sql);
    }

    public ResponseObject OutputAsDataSet(string sql)
    {
         return DAI.GetDataSet(_db, sql);
    }

    public ResponseObject OutputAsDataTable(string sql)
    {
         return DAI.GetDataTable(_db, sql);
    }
}

Now, the only two things I see as an advantage is that the nomenclature is more clear when wrapped up in a non-static container, and that there are less parameters to pass around.

But I'm wondering if this is a good idea by design to wrap up static class with non-static? What are some of the other reasons if there are any? Because I assumed that creating a static and making calls to it would be okay. But this project has made it a deliberate point to wrap up all static class; and I'm not sure why.

Answer 1

The most common reason I've done something like this in the past is if the static methods are provided by a third-party library (i.e. I didn't write it), but I don't want to write code that takes a direct dependency on that library. In that case, I'll write my own class and have it take the direct dependency instead.

Assuming I use an interface (or something similar like in your example), then if I decide down the road that I want to use a different library, I can write another class that implements the same interface and swap out the concrete class at runtime (using something like Dependency Injection).

Answer 2

Looks to me like they are trying to make the object injectable in order to make your code easier to test and to modify later.

check this post: Why does one use dependency injection?

Answer 3

Personally I would never do this, what I would do instead is use a .NET dependency injection framework like Ninject or Unity in order to inject the necessary references into objects as they're being created. There are many advantages to doing this...for starters you can create your singleton objects without actually having to use static members and you have much more control over their life cycle. If you want to do unit testing (and you should) then it is trivial to replace your singleton with a mocked object, and if you decide later on that maybe a singleton isn't the best design choice then it's trivial to inject an object that is scoped to something else such as the main parent view model etc. The project you're looking at is probably using that intermediate class to enable some of the things I'm talking about, but it'll never be as flexible or clean as doing proper dependency injection.

Answer 4

It's a standard pattern. - Wrap a more complicated set of methods to hide their complexity, such as a set of methods which only return errors via thrown exceptions. - Handle errors, exceptions, etc. in the wrapper methods - Make the methods static to prevent multiplication of derived methods like executeQueryByInt, executeQueryByLong, executeQueryByString, ... - Limit the propagation of references to the SQL handling framework code to only the wrapped methods - important Have a single place to document how to call, errors, special cases, bug workarounds when using the third party static library

For unit testing, your unit tests should implement a short wrapper class which only passes through calls to the static class.

There's no need to add another layer of complexity, no matter how simple, to just fit an arbitrary pattern or cloud your code. Your production code should not implement extra code just if that code is used in unit tests.

It's common to wrap a library or nuget package in a project by itself so that a multi-solution project does not have dozens of package references to a third party package.

Odd how Angular has the concept of a barrel, yet .net core has none; and you get into Nuget Package h*ll once the number of projects is more than 10 and the solution is older than 2 years.

Having restructured 1,000,000+ line .net solutions multiple times, static methods and static classes make the code easier to move around in the larger context. What best practices scale well at smaller projects do not scale well for 1,000,000+ line systems. It's a different approach than found in toy projects and short blog entries. It's about being able to see what code is called and where the code is called so that refactoring, simplification is easier.

Answer 5

The added class ExecSP is serving no benefit. Both methods pass in a db string and a sql string. I imagine the db string is some sort of connection string to a database instance and the sql is the raw sql string to be executed by that instance. In this case might as well call the DAI directly, there is no reason for the wrapper class.

From a design prosepctive, this is tight coupling. We probably want to abtract away the database (IDatabase) by creating an instance and then running a command against the abstraction instead of passing around the connection string/sql statement.

Psudeo Code:

IDatabase dbInstance =  new DatabaseCreator(db);
dbInstance.Execute(sql); 

Then it doesn't matter if the database is SQL Server, Oracle, etc.

Abstraction helps with testing. For example, in a unit test project I can write my own IDatabase implementation that doesn't even use a database. I am not sure how you would test this without an actual database instance. If I write my own test instance, I can remove that external dependency.