I currently have a method in my repository that will run SQL and map the reader to objects:protected IEnumerable<T> Query<T, TFactory>(string sql, List<IDbDataParameter> parameters = null) where TFactory : IFactory<T>{    List<T> list = new List<T>();    var factory = Activator.CreateInstance<TFactory>();    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<Code> 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<IDbDataParameter>();    parameters.Add(DbFactory.GetParameter(:1, CodeType, DbType.String));    parameters.Add(DbFactory.GetParameter(:2, Code, DbType.String));    return this.Query<Code, CodeFactory>(sql, parameters);}I would love to hear thoughts and feedback on how I can improve.
Running SQL and mapping the reader to objects
c#;asp.net
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 shouldnt use optional parameters, they force you to recompile all when you change them.Instead, use an overloaded method as follow:protected List<T> Query<T, TFactory>(string query) where TFactory : IFactory<T>{    return Query<T, TFactoty>(query, Enumerable.Empty<IDbDataParameter>());}protected List<T> Query<T, TFactory>(string query, IEnumerable<IDbDataParameter> parameters)     where TFactory : IFactory<T>{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<T> Query<T, TFactory>(string query) where TFactory : IFactory<T>{    return Query<T, TFactoty>(query, Enumerable.Empty<IDbDataParameter>());}protected List<T> Query<T, TFactory>(string query, IEnumerable<IDbDataParameter> parameters)     where TFactory : IFactory<T>{    var collection = new List<T>();    var factory = Activator.CreateInstance<TFactory>();    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 wont 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.