How can I design this better? (Avoiding a switch s

Question

I know a little bit about Object Oriented design, but I'm not sure how to go about using those principles in my code. Here's what I'm working on:

    public void Query(Agency agency, Citation queryCitation) {
        queryCitation.AgencyCode = agency.AgencyCode;

        switch (agency.ClientDb.Type) {
            case "SQL":
                QueryOracle(agency, queryCitation);
                break;
            case "PIC":
                QueryPick(agency, queryCitation);
                break;
        }
    }

(Most of these are objects from NHibernate. I'm working with a legacy database system and am refactoring parts of it into a code library.) Clearly, I could do something differently here so that I don't need duplicate functions for different database queries that have the same input. It should just know based off of the agency object whether to use an Oracle database or a Pick database connection. (If you've never heard of a Pick database, well I hadn't either until I started working here. We make queries to it through HTTP requests, so it's not SQL.)

Should I make an interface, for example called "ClientDbConnection" and then make two classes that implement that interface, move the code to query the database to those and then have something like "agency.clientDb.Query(queryCitation)" replace this whole function? I guess I'm thinking aloud here, but any input on this would be appreciated.

Answer 1

ADO.NET has a set of generic classes: DbCommand, DbConnection, etc... that also implement another set of generic interfaces: IDbCommand, IDbConnection, etc...

So you could use them, but it may become quite complicated in the end. The advantage of your solution is it's very readable. Plus maybe the pick database does not have any ADO.NET provider...

PS: I would replace the Type property type though, and use an enum instead.

Answer 2

There are two OOP solutions polymorphism and Visitor pattern.

Answer 3

I would refactor to take advantage of an interface. I would probably make it something like:

public interface IQuery
{
    void Execute(Agency agency, Citation query);
}

public class OracleQuery : IQuery
{
    // Implementation
}

public class PickQuery : IQuery
{
    // Implementation
}

You could then change the Agency class to store an instance of an IQuery object rather than (or in addition to) the ClientDb object:

public class Agency
{
    public IQuery Query { get; set; }
}

And then in your initialization code (where you would normally set the ClientDb property), you could set the instance to the appropriate IQuery implementation:

agency.Query = new PickQuery();

Answer 4

You will likely want to implement the strategy pattern here. Basically, each of your possible "types" in your switch statement would become a class of its own that implements the same interface.

Applying the Strategy Pattern

You can then use a factory method that takes a "type" value as its parameter. That method would return the correct class (its return type is the interface mentioned above).

Answer 5

To write less code but improve readability the the level of declaritive code you can swith to a dictionary with delegates for each database. That can easily be extended and is very readable. With that more functional approach in mind you get something like

void Query(Agency agency, Citation queryCitation)
{
    Dictionary<string, Action<Agency, Citation>> QueryMap = new Dictionary<string, Action<Agency, Citation>>
    {
        { "SQL", QueryOracle},
        { "PIC", QueryPic}
    };


    queryCitation.AgencyCode = agency.AgencyCode;

    QueryMap[agency.ClientDb.Type](agency, queryCitation);
}

Answer 6

Is Agency a class you control? If so do something like this:

public abstract class GenericDb
{
    public abstract void Query(parms);
}

In your Agency Class, you could have

public GenericDb ClientDb {get; set;}

Then have a SqlDb class like:

public class SqlDb : GenericDb
{
    public void Query(parms);
}

public class PicDb : GenericDb
{
    public void Query(parms);
}

Then this code:

public void Query(Agency agency, Citation queryCitation) {
        queryCitation.AgencyCode = agency.AgencyCode;

        switch (agency.ClientDb.Type) {
            case "SQL":
                QueryOracle(agency, queryCitation);
                break;
            case "PIC":
                QueryPick(agency, queryCitation);
                break;
        }
    }

becomes

public void Query(Agency agency, Citation queryCitation) {
        queryCitation.AgencyCode = agency.AgencyCode;
        agency.ClientDb.Query(queryCitation);
    }

Because of inheritance, it will know that ClientDb has a base class of GenericDb. It will know by the type of the ClientDb parameter whether it should run the SqlDb or the PicDb or Oracle etc.