Running SQL and mapping the reader to objects

I currently have a method in my repository that will run SQL and map the reader to objects:

protected IEnumerable Query(string sql, List parameters = null) where TFactory : IFactory
{
    List list = new List();

    var factory = Activator.CreateInstance();
    var connection = (Db == null) ? Connection : Db.Connection;

    using (var manager = new DbCommandManager(connection, sql))
    {
        if (parameters != null)
            foreach (var parameter in parameters)
                manager.AddParameter(parameter);

        using (var reader = manager.GetReader())
        {
            while(reader.Read())
                list.Add(factory.CreateTFromReader(reader));
        }
    }

    return list;
}

To use the following you simply do:

public IEnumerable GetCodeWType(string CodeType, string Code)
{
    var sql = @"Select CODE_TYPE, CODE, DESCRIPTION From Codes  Where (CODE_TYPE = :1) AND (CODE = :2)";

    var parameters = new List();
    parameters.Add(DbFactory.GetParameter(":1", CodeType, DbType.String));
    parameters.Add(DbFactory.GetParameter(":2", Code, DbType.String));

    return this.Query(sql, parameters);
}

I would love to hear thoughts and feedback on how I can improve.

It looks ok for me so, just a couple of suggestions:

First: given it is a protected method and not a private, it can be called be derived classes which probably could not be in the same assembly (this is true only if the class is public) then, you shouldn´t use optional parameters, they force you to recompile all when you change them.

Instead, use an overloaded method as follow:

protected List Query(string query) where TFactory : IFactory
{
    return Query(query, Enumerable.Empty());
}

protected List Query(string query, IEnumerable parameters) 
    where TFactory : IFactory
{

It also help you to remove the checking for nulls.

Second, if you return an IEnumerable, the callers will have less features. The rule is you you should require the most generic and return the most specific implementation so, if you have a list you could return a List<>, IList<> or a Collection. Then, callers have more options.

My attempts is this one:

protected List Query(string query) where TFactory : IFactory
{
    return Query(query, Enumerable.Empty());
}

protected List Query(string query, IEnumerable parameters) 
    where TFactory : IFactory
{
    var collection = new List();

    var factory = Activator.CreateInstance();
    var connection = (Db == null) ? Connection : Db.Connection; // this line looks rare for me

    using (var manager = new DbCommandManager(connection, sql))
    {
        foreach (var parameter in parameters)
            manager.AddParameter(parameter);

        using (var reader = manager.GetReader())
        {
            while (reader.Read())
                list.Add(factory.CreateTFromReader(reader));
        }
    }

    return list;
}

Update:

One more thing about optional parameters, when you call the Query method with one argument, you really are passing two: the query and a null in 'parameters'. You have never pass nulls (or at least you have to try) because if you pass nulls then, you need to check for null values.

One more thing about the protected modifier, privates methods can trust that their parameter won´t be null but protected methods cannot do it. This is because private members will be called by other methods in the same class but protected method can be called by other methods in other classes.

StackExchange Code Review Q#19500, answer score: 2