Opinions on inheriting from utility class

N

Nerseus

Danner
Joined
Oct 22, 2002
Messages
2,547
Location
Arizona, USA
We have a small debate at work and I'm wondering if there's any information on going one way or the other on this.

The setup is that there exists a database utility class - a class that serves only to expose some static methods. It looks something like this:
C#: 
public class DataAccess
{
    protected static readonly string connectionString;

    static DataAccess()
    {
        connectionString = ""; // Code ommitted - assumes it gets the connection string
    }

    internal static DataSet ExecuteNonQuery(string commandText)
    {
        // Code to create a DB command object, set the connection string
        // and call ExecuteNonQuery
        // Also handles exception handling by logging errors and other misc details
    }
}

So the general syntax for using the static method is:
DataAccess.ExecuteNonQuery("procName 'hello world'");

Now the argument comes up over whether a specific Data Access Layer class should inherit from this class or not. Here's a sample that does inherit:
C#: 
internal sealed class CustomerDataAccess : DataAccess
{
    // Private constructor so it can't be created - in 2005 you'd declare the 
    // class as static
    private CustomerDataAccess() {}

    public static void SaveCustomer(DataSet ds)
    {
        ExecuteNonQuery("UpdateCust '" + ds.GetXml() + "'");
    }
}

Again, ignore the syntax of passing params to a stored proc as a string without any SQL Injection checks.

In the above, I'd rather NOT have CustomerDataAccess inherit from DataAccess. The only change is that the call to ExecuteNonQuery changes to:
C#: 
    public static void SaveCustomer(DataSet ds)
    {
        DataAccess.ExecuteNonQuery("UpdateCust '" + ds.GetXml() + "'");
    }

But others argue that by inheriting from DataAccess, you're declaring your intentions that this is a DataAccess component and the method calls "look" like they're part of CustomerDataAccess.

I'd argue that they're NOT part of CustomerDataAccess but that they're still part of DataAccess since they're static.

I guess I don't see the benefit of inheritance in this case a both classes are really utility classes.

Any thoughts?

-nerseus
 
Personally I wouldn't use inheritance in this case, the way I see it is that the CustomerDataAccess class uses the DataAccess class (specifically it's methods) but it isn't a more specific form of the DataAccess class.

If inheritance is used every XXXXDataAccess class will also be exposing the DataAccess class' functionality and that doesn't seem right.
Plus I always feel I'm doing something wrong when I'm inheriting from static classes anyway....

I would take the approach that
that by inheriting from DataAccess, you're declaring your intentions that this is a DataAccess component and the method calls "look" like they're part of CustomerDataAccess.
Click to expand...
is taking an incorrect view of the system - the DataAccess is exposing functionality to access the DB in a general way; the CustomerDataAccess is exposing specific functionality for the DB in regard to customers.
If the CustomerAccessClass exposes (even as internal) generalised methods like ExecuteNonQuery then the programmatic model says it is okay to execute arbitrary code against the DB (possibly not even relating to customers) from the Customer component - this seems very wrong to me.

I would imagine the exposed methods like SaveCustomer(DataSet ds) are doing parameter validation etc. which takes into account Customer specific requirements unlike the more general ExecuteNonQuery and related methods which don't (and shouldn't) have any knowledge of the higher classes - this can only increase the potential for errors.
Also if your system is using unit tests then a class like CustomerDataAccess can have very specific tests written for each of the methods it exposes, non-specific methods are just a back-door where any 'short cut', 'optimised' or 'nasty hack' bits of code can bypass the documented and correct interface for the component.

Just my 2p worth...
 
Last edited:
One thing that I like about Visual Basic is that you can import (C#'s using statement) not only namespaces but also static members of classes into scope so that the code could look the same either way. Unfortunately, for some reason, we can't do this in C#.

My take on this situation is that if you were to inherit a utility class to save yourself some typing, you are abusing inheritance. That is, you are using it for something very different from what it is meant to do. Inheritance is obviously not there to bring functions into scope without qualifying them, and the reason that this may be a concern is confusion as to whether a function is static or instance and how it may play into the inheritance between DataAccess and CustomerDataAccess (PlausiblyDamp makes a good case for this). I don't think that it is a big deal either way (it only takes a second to check the function definition), but it may lead to mild confusion if you use inheritance, or any feature for that matter, for one thing when it is meant for another.

On the other hand, if the DataAccess provides a number of abstract members and unifies some of the funtionality of your DataAccess classes, then DataAccess simply becomes a convinient place to put all your general data access functions (not that one should create some abstract members purely for the sake of justifying inheritance of that class). For example, perhaps it would make more sense to have an abstract SaveItem in the DataAccess class and override it in the CustomerDataAccess class, provided that doing so would have some practical benefit.
 
Thanks for the replys! Your replies were right on and solidify what I was thinking - much better argued than I was thinking. I'm glad I asked!

-ner
 
I just found this article on Wikipedia involving Anit-Patterns and the situation you described here. It basically backs up what all of us were thinking about this not being the best idea to do.
 
Back
Top