Implementation of the Strategy Pattern using an interface, abstract class and delegate

The following class was designed to help create a more detailed error message than what's provided by the repository when a user tries to insert text into a column that is > the column max length. The users would like to be able to track down exactly which column(s) were the issue. Therefore, I created the following, and I would like to know if this is a good approach or not.

This class defines the parameters needed and is the context object for each respective strategy class.

public class QueryBuilder
{
public StringBuilder ColumnNames{get;private set;}
    public StringBuilder ColumnValues{get;private set;}
    public string TableName{get;private set;}
    public Database.FactoryType FactoryType {get;private set;}

    private static Dictionary queryBuilderStrategies;
    public QueryBuilder(StringBuilder columnNames,StringBuilder columnValues,string tableName, Database.FactoryType factoryType)
    {
            TableName = tableName;
            ColumnNames = columnNames;
            ColumnValues = columnValues;
            FactoryType = factoryType;

            LoadStrategies();
    }
    private void LoadStrategies()
    {
        queryBuilderStrategies = new Dictionary();
        queryBuilderStrategies.Add(Database.FactoryType.SqlClient, new SQLErrorQueryStrategy() );
        queryBuilderStrategies.Add(Database.FactoryType.OleDb, new AccessErrorQueryStrategy());
    }
    public string Create()
    {     
            return queryBuilderStrategies[FactoryType].CreateSQLQueryString(this);
    }

    public string Read(IDataReader reader)
    {
        return queryBuilderStrategies[FactoryType].ParseDataReader(this, reader);
    }
}

I created the following interface to decouple all the different strategies needed to communicate with multiple databases that we support.

```
public interface IQueryBuilderStrategy
{
string CreateSQLQueryString(QueryBuilder queryBuilder);
string ParseDataReader(QueryBuilder queryBuilder, IDataReader reader);
}

I think QueryBuilder can use constructor parameters, and that its auto-properties can be changed to be { get; private set; }. Otherwise the public setters seem to make an opportunity for some trouble. Actually I'd drop the auto-properties and initialize private readonly fields, exposed by get-only properties.

I don't get the static modifier for a dictionary that is recreated for every instance of the type. I would drop the static modifier altogether.

public Database.FactoryType Type {get;set;}

Might be a nitpick, but I find Type is a confusing name to use here, it clashes with System.Type. FactoryType would be much more appropriate.

Also there are inconsistencies in the way you're dealing with IDisposable in the try block of GetDetailedErrorMessage:

DbCommand errorQueryCommand =
        database.CreateCommand(queryBuilder.Create(), CommandType.Text);            
    errorQueryCommand.Connection = databaseConnection;          
    using (DbDataReader reader = errorQueryCommand.ExecuteReader(CommandBehavior.SchemaOnly))
    {
        message += queryBuilder.Read(reader);                             
    }
    if (errorQueryCommand != null)
        errorQueryCommand.Dispose();

I don't see a reason not to do using (var errorQueryCommand = database.CreateCommand(...); also you're assigning the command's Connection to some databaseConnection which has to be a private field (that would be clearer if the name was _databaseConnection instead, but that could be only me).

One thing though, is that I don't think errorQueryCommand would ever be null where you're testing for it - if it were the case, you'd already be in the catch block over a NullReferenceException caused by accessing the setter of errorQueryCommand.Connection. But that point is moot if you wrap the DbCommand in a using block.

The rest looks ok, except if you're a fan of var, in which case reading your code... tickles. I personally find it very redundant to see string message = string.Empty; when it's obvious that message is a string - not because of its name, but because of the value it's being assigned to.

StackExchange Code Review Q#39844, answer score: 7