0

Update: Most answers are about how to write a wrapping interface or what is a good wrapper around ADO. but my question is more about when to do so and when not.

I never found a good rule for deciding when to write a wrapper and when not. As nobody came up with a suggestion I think the reviewers are right in that this question has no general answer and is to broad.

Even in the specific case of ADONET the answer may depend on the use case and for most use cases a wrapper has been written already. Dapper and Orseis for e.g. as MainMa points out.

I would like to remove the question but I do not want to steel any ones points 8-)

Original Question

Let's take code executing a query using ADONET as an example.

var cmd = connection.CreateCommand();
cmd.CommandText = "SELECT 1 FROM DUAL";
cmd.CommandTimeout = 1000;
var dataReader = cmd.ExecuteReader();

Many will want to write this as a one-liner and create a bunch of utility functions in this example called as follows (or extension classes):

var cmd = connection.CreateCommand();
Utility.SelectToReader (connection, "SELECT 1 FROM DUAL", 1000);

So what are good rules to introduce or deprecate utility functions? Apart from ADONET this question arises using almost any framework.

closed as too broad by gnat, jwenting, amon, pdr, GlenH7 May 7 '14 at 19:28

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • If you need a utility function to wrap the usage of another function, that is a sign that whatever you are wrapping was poorly designed. For a built-in function there is not much you can do though. I guess it could be valid in the context of the Adapter pattern, but I do not think that is what is going on here. – user22815 May 6 '14 at 19:43
  • @JohnGaughan, and what is a sign that I need to wrap an interface (appart from the case of an adapter as you noticed)? – Patrick Fromberg May 6 '14 at 20:00
  • If the interface is really klunky and tedious to use. – user22815 May 6 '14 at 20:06
  • @JohnGaughan Not necessarily. The wrapper could be for a specific, common use of a flexible system. For example, such as in the question, SQL that takes no parameters is a common but specific subset of SQL in general. – Izkata May 6 '14 at 23:31
  • 1
    In the example in the question, there is a simple factory method that creates a command. The caller then sets properties, and calls an "execute" method. There are conflicting ideologies on this: should it be a single function call? Is the current way correct? My point is that regardless of the decision made, the interface needs to be elegant, intuitive, and easy to use. – user22815 May 7 '14 at 5:40
5

John Gaughan already said it, "If you need a utility function to wrap the usage of another function, that is a sign that whatever you are wrapping was poorly designed."

Indeed, ADO.NET is old and requires to write much boilerplate, inelegant code which can easily cause mistakes (like forgetting to open a connection before starting a query or forgetting to complete a transaction when finishing working with it).

You may start doing utility functions. But remember, C# is object oriented, so you may want to use a more conventional way of a library. You may create your own, but why reinventing the wheel? There are already plenty of libraries which abstract ADO.NET calls and provide a much better interface. There are many ORMs, including Entity Framework and the much more lightweight LINQ to SQL, and if an ORM is an overkill for your current project, why not using something like Dapper?

What's wrong with utility functions, you may ask?

Nothing, except that there is no benefit whatsoever compared to an object-oriented approach of a library, and that you lose all you get with OOP. Let's see what it gives on an example of Orseis, a library similar to Dapper (but much, much better, because I created it; nah, just joking).

In this library, database is accessed this way:

var numberOfProducts = 10;
var query = @"select top (@Limit) [Id], [Name]
              from [Sales].[Product]
              order by [Sold] desc";

var products = new SqlDataQuery(query, this.ConnectionString)
    .Caching(TimeSpan.FromMinutes(5), "Top{0}Sales", numberOfProducts) // Cache the results.
    .Profiling(this.Profiler) // Profile the execution of the query.
    .TimeoutOpening(milliseconds: 1000) // Specify the timeout for the database.
    .With(new { Limit: numberOfProducts }) // Add query parameters.
    .ReadRows<Product>();

This syntax has several benefits:

  1. It's slightly much more readable and intuitive than the way it would be written using utility functions:

    var numberOfProducts = 10;
    var query = @"select top (@Limit) [Id], [Name]
                  from [Sales].[Product]
                  order by [Sold] desc";
    
    using (Utility.OpenConnection(this.ConnectionString, timeoutMilliseconds: 1000))
    {
        var parameters = new { Limit: numberOfProducts };
        Utility.Cache(
            TimeSpan.FromMinutes(5),
            "Top{0}Sales",
            numberOfProducts,
            () => Utility.Profile(
                () =>
                {
                    using (var reader = Utility.Select(connection, query, parameters))
                    {
                        var products = Utility.ConvertAll<Product>(reader);
                    }
                }
            )
        );
    }
    

    I mean, how a new developer could possibly understand what's happening here in just a few seconds?

    It's not even about naming conventions (not that I want to criticize your choice of Utility as a name for a class), but about the structure itself. It simply looks like the plain old ADO.NET.

  2. It's refactoring friendly. I can reuse a part of a chain (I can do it too with utility functions) very easily during a refactoring (that's much harder with utility functions).

    Imagine that in the previous example, I want to be able to specify profiling and timeout policy once, and reuse it everywhere. I'll also specify the connection string:

    this.BaseQuery = new SqlDataQuery(this.ConnectionString)
        .Profiling(this.Profiler)
        .TimeoutOpening(milliseconds: 1000);
    
    // Later in code:
    
    var numberOfProducts = 10;
    var query = @"select top (@Limit) [Id], [Name]
                  from [Sales].[Product]
                  order by [Sold] desc";
    
    var products = this.BaseQuery
        .Query(query)
        .Caching(TimeSpan.FromMinutes(5), "Top{0}Sales", numberOfProducts)
        .With(new { Limit: numberOfProducts }) // Add query parameters.
        .ReadRows<Product>();
    

    Such refactoring was pretty straightforward: it just works. With the variant using utility functions, I could have spent much more time trying to refactor the piece of code without breaking anything.

  3. It's portable. Adding support for a different database, such as Oracle, is seamless. I can do it in less than five minutes. Wait, I already did, and it took 5 lines of code and less than a minute (the time needed to install Oracle doesn't count, right?).

    This one is crucial, and this is also why .NET developers got it wrong in .NET 1 when they designed File and Directory utility functions. Imagine you've created an app which spends a large deal of time working with files. You're preparing for your first release the next week when you receive a call from the customer: he wants to be able your app to work with Isolated Storage too. How do you explain to your customer that you'll need additional two weeks to rewrite half of your code?

    If .NET 1 was designed with OOP in mind, they could have done a file system abstract provider with interchangeable concrete providers, something similar to another project I started, but not finished yet. Using it, I can seamlessly move from file storage to Isolated Storage to in-memory files to a native Win32 storage which supports NTFS transactions and doesn't have the stupid .NET constraint of 259 characters in a path.

  4. It's Dependency Injection (DI) friendly too. In point 2, I extracted a part of a chain in order to reuse it. I can as well push it even further and combine it with DI. Now, the methods which actually do all the work of querying the database don't even have to know that I'm using Oracle or SQLite. By the way, they don't have access to the query string; it's by design.

  5. It's easily extensible. I had to extend Orseis dozens of times, adding caching, profiling, transactions, etc. If I had to struggle with something, it was more the features themselves and how to make them fool-proof. For example, the notion of collection propagations I implemented to seamlessly bind queries containing joins to collections of objects wasn't a good idea, because despite all my efforts, it's still not obvious to use and can be a source of lots of mistakes.

    But adding simpler concepts (like cache invalidation of multiple entries or the reusing of a connection in multiple queries) was pretty straightforward. This straightforwardness is much more difficult to find in utility functions. There, you start adding something, and you find that it breaks consistency or requires to make the changes which are not backwards compatible. You end up creating a bunch of static methods which are so numerous, that they look more like patchwork than a consistent class which helps developers.

    Let's take your example:

    Utility.SelectToReader(connection, "SELECT 1 FROM DUAL");
    

    Later, you need to use the parameters, so it becomes:

    Utility.SelectToReader(
        connection,
        "SELECT 1 FROM DUAL WHERE CATEGORY = @Category",
        new Dictionary<string, string>
        {
            { "Category", this.CategoryId },
        });
    

    Then, you notice that you have too many timeouts, so you must add a timeout too:

    Utility.SelectToReader(
        connection,
        "SELECT 1 FROM DUAL WHERE CATEGORY = @Category",
        new Dictionary<string, string>
        {
            { "Category", this.CategoryId },
        },
        2500);
    

    Step by step, the method becomes unusable. Either you need to split it, or you keep one with a dozen of optional arguments.

  • The question is more about how to implement Dapper as opposed using Dapper as an ORM layer. But good idea because Dapper is open source, so I can have a look at that on the weekend. If Dapper is good, its implementation is hopefully good too. – Patrick Fromberg May 7 '14 at 7:29
  • No complaint about your answer in general but the OO remark is wrong. If a solution is good but not OO, thats totally ok. If a stateless utility class is visibly stateless, thats good at least for readability. – Patrick Fromberg May 7 '14 at 7:55
  • @PatrickFromberg: I edited my answer to explain the OOP issue. – Arseni Mourzenko May 7 '14 at 10:58
  • Thanks. This is a lot of info. I will look into it on the weekend. My impression so far is that cbojar may be very right with his remark about Fallacy. – Patrick Fromberg May 7 '14 at 12:15
  • @PatrickFromberg: that's obvious. But remember, ADO.NET is very old, and was created when .NET wasn't as mature as it is today. This also explains why ADO.NET, as is, is so unusable, as well as the efforts of Microsoft to convince everyone to use Entity Framework and other ORMs instead. – Arseni Mourzenko May 7 '14 at 13:17
3

The biggest fallacy in your question is that vendors think that hard about their APIs. :) I (half) kid, but truthfully, vendors don't always write the best APIs. Sometimes this is to provide ultimate low-level flexibility. Other times this is to meet obscure or otherwise unknowable requirements. Yet other times its because vendors don't totally know or understand how developers are going to use their code. As an example, a number of languages provide straightforward APIs for reading XML from strings and manipulating it, but getting the result back out as a string is a chore.

In cases where you have this much boilerplate to perform a simple action, you can use the facade pattern to abstract away the complexities. Your utility methods are partway there, but having a single utility class like that is not the ideal approach. Such a class suffers from low cohesion, since it can have all kinds of unrelated methods. It would be better to create highly cohesive facade classes of related functionality. Highly cohesive classes make duplicate code or deprecated functionality more apparent because you are looking at code all in a similar vein.

  • I agree that the facade is for that. But if utility methods are a good idea, they should be the least intrusive possible I think. Hence if utility needs not to deal with state it should be just a bunch of functions. – Patrick Fromberg May 7 '14 at 7:34
  • Utility methods should be as unobtrusive as possible. Small, focused facade classes grouping tightly related functionality are far less intrusive than a demigod-like global utility class. Small classes have a small area to mock, can be DI'ed, clearly identify the nature of a dependency, can grow fluidly into a more comprehensive abstraction, and can be more easily managed. As an example, if you wanted to create a lib from a part of your project that depends on a utility class, is it easier to depend on a small class or a giant that has a smattering of functionality from across your app? – cbojar May 8 '14 at 4:01

Not the answer you're looking for? Browse other questions tagged or ask your own question.