Generic database select method for within my application

Question

I want to implement a generic Get method for my ASP MVC application, which gets data from an MS Access database.

This is what I have so far, where I am testing it on the Pallet object.

Access

Get

public List<T> Get<T>(List<string> parameters)
{
 query = DatabaseConstants.Select<T>();
 argNames = DatabaseConstants.SelectParameters<T>();
 args = new DynamicParameters();
 foreach (var p in parameters)
 {
 args.Add(argNames[parameters.IndexOf(p)], p);
 }
 return NewSelect<T>(query, args);
}

Select

public List<T> NewSelect<T>(string query, DynamicParameters args = null)
{
 using (var db = new OleDbConnection(connectionString))
 {
 return (List<T>)db.Query<T>(query, args);
 }
}

DatabaseConstants

SelectQuery

public static string SelectQuery<T>()
{
 if (typeof(T) == typeof(Pallet))
 {
 return "SELECT * FROM subGRVReceived WHERE GRVNo=@grv";
 }
 else if (typeof(T) == typeof(Sample))
 {
 return "SELECT * FROM subReceivingQC WHERE GRV=@grv AND PalletSeq=@pallet"; 
 }
 else if (typeof(T) == typeof(QCItem))
 {
 return "SELECT * FROM subQCItems WHERE GRV=@grv AND PalletSeq=@pallet AND SampleNo=@sampleno";
 }
 else
 {
 return string.Empty;
 }
}

SelectParameters

public static List<string> SelectParameter<T>()
{
 if (typeof(T) == typeof(Pallet))
 {
 return new List<string>() { "@GRV" };
 }
 else if (typeof(T) == typeof(Sample))
 {
 return new List<string>() { "@grv", "@pallet" };
 }
 else if (typeof(T) == typeof(QCItem))
 {
 return new List<string>() { "@grv", "@pallet", "sampleno" };
 }
 else
 {
 return new List<string>();
 }
}

The method can be called like this:

Pallets = access.Get<Pallet>(new List<string>() { grvNumber });

Once I have completed this I will also look into creating generic Insert, Update etc methods. my main goal here is to just reduce the amount of code in the project, as I currently have a seperate method for each database query.

Does anyone have any thoughts or suggestionss on this? Many thanks for taking the time to look!

___________-

I initially wanted to implement an IDatabaseObject containing Query and ParameterNames, where Pallet can inherit from this and store the values inside itself, but I got an error, as the property Pallets is of type List<Pallet>, and C# does not seem to allow me to assign the output of Get<T> to this value when I included a where T : IDatabaseObject.

• t3chb0t: Could you also include these parts: // if/else for other types?
• Bassie: @t3chb0t I have not implemented these yet, but they will just be similar to the first, e.g. if (typeof(T) == typeof(Sample)) { return "some other query string"}; Would you still like me to include some examples?
• t3chb0t: If you have examples then yes but please don't invent anything for the purpose of the question. You'll get better answers if you post exactly what you have. I'm just making sure it won't get closed for code-not-yet-written ;-) As far as I'm concerened it's currently somewhere inbetween, neither entirely off-topic nor on-topic. Especially this sentence Once I have completed this makes me worry. I think it would be more realistic if you included more examples or types that you are going to implement into this if/else pattern later so that we can see how similar they are etc.
• t3chb0t: Ok, I see you edited the question at the same time as I posted the comment. It looks much better now. I think you are going to get much better answers.
• Bassie: @t3chb0t I added some examples, but while doing so I realised that I'm actually going to need to query the same table but with an extra parameter at some point, so this probably wont work .. However as it stands it could be fairly useful (although not as reusable or generic as I might like!)

Answer 1

First things first: your current code has a huge optimization to be made.

foreach (var p in parameters)
{
 args.Add(argNames[parameters.IndexOf(p)], p);
}

This is wrong, because argNames has an indexer and parameters is a List<string>. The IndexOf(T) method you call on parameters to get the index increases the amount of effort, work and time that your program has to perform in order to give you a result when all you really need to do is index the list.

for (int i = 0; i < parameters.Count; i++) 
{
 args.Add(argNames[i], parameters[i]);
}

Much simpler. Of course, if the parameters list is larger than argNames, it will throw an exception. You can either keep it that way, or throw and explicit exception (which you should do to inform the user why their code failed).

public List<T> Get<T>(List<string> parameters)
{
 argNames = DatabaseConstants.SelectParameters<T>();
 if (argNames.Length != parameters.Count) // Is argNames a List? If so replace `Length` with `Count`
 {
 throw new ArgumentException($"The argument provided for '{nameof(parameters)}' was not the same length as the required number of parameters for this query.");
 }
 query = DatabaseConstants.Select<T>();
 args = new DynamicParameters();
 foreach (var p in parameters)
 {
 args.Add(argNames[parameters.IndexOf(p)], p);
 }
 return NewSelect<T>(query, args);
}

---

Next, let's rethink your process.

public static string SelectQuery<T>()
{
 if (typeof(T) == typeof(Pallet))
 {
 return "SELECT * FROM subGRVReceived WHERE GRVNo=@grv";
 }
 else if (typeof(T) == typeof(Sample))
 {
 return "SELECT * FROM subReceivingQC WHERE GRV=@grv AND PalletSeq=@pallet"; 
 }
 else if (typeof(T) == typeof(QCItem))
 {
 return "SELECT * FROM subQCItems WHERE GRV=@grv AND PalletSeq=@pallet AND SampleNo=@sampleno";
 }
 else
 {
 return string.Empty;
 }
}

This screams for a proper ORM, or at least a fake implementation of one. You also mention in the comments:

I added some examples, but while doing so I realised that I'm actually going to need to query the same table but with an extra parameter at some point, so this probably wont work .. However as it stands it could be fairly useful (although not as reusable or generic as I might like!)

What if I told you that not only could we make this generic (and generate the query dynamically), but we could solve that problem at the same time?

I'm going to solve them one-at-a-time, but it won't be too difficult.

First, we need two pieces of data. We need the table name and property name, assuming that they're not defined by the property name itself. (As it appears.)

If you have access to the System.ComponentModel.DataAnnotations library, and the System.ComponentModel.DataAnnotations and System.ComponentModel.DataAnnotations.Schema namespaces, then this is significantly easer, otherwise you need to define the three attributes we're going to use. I'm going to use built-in's.

First, we're going to define our keys on the object with the System.ComponentModel.DataAnnotations.Key attribute, then we'll define the System.ComponentModel.DataAnnotations.Schema.Table attribute, and the System.ComponentModel.DataAnnotations.Schema.Column attribute.

[Table("TestTable")]
public class Foo
{
 [Key]
 [Column("fooId")]
 public int Id { get; set; }
 public string Name { get; set; }
}
[Table("TableBar")]
public class Bar
{
 [Key]
 [Column("barId")]
 public int BarId { get; set; }
 [Column("barName")]
 public string Name { get; set; }
}
public class Baz
{
 [Key]
 public int Id { get; set; }
 public int Order { get; set; }
}

The result is going to be something like:

SELECT * FROM TestTable WHERE fooId = @fooId
SELECT * FROM TableBar WHERE barId = @barId AND Name = @Name
SELECT * FROM Baz WHERE Id = @Id

So, how do we get there? A lot of reflection.

public static List<string> GetParameters<T>(IEnumerable<string> additionalConstraints = null)
{
 var type = typeof(T);
 var result = new List<string>();
 var keys = type.GetProperties().Where(x => x.IsDefined(typeof(KeyAttribute), true));
 if (keys.Any())
 {
 foreach (var key in keys)
 {
 if (key.IsDefined(typeof(ColumnAttribute), true))
 {
 var nameAttr = (ColumnAttribute)key.GetCustomAttributes(typeof(ColumnAttribute), true).FirstOrDefault();
 result.Add($"@{nameAttr.Name}");
 }
 else
 {
 result.Add($"@{key.Name}");
 }
 }
 }
 if (additionalConstraints?.Any() == true)
 {
 foreach (var constraint in additionalConstraints)
 {
 var prop = typeof(TableAttribute).GetProperties().Where(x => x.Name == constraint).First();
 if (prop.IsDefined(typeof(ColumnAttribute), true))
 {
 var nameAttr = (ColumnAttribute)prop.GetCustomAttributes(typeof(ColumnAttribute), true).FirstOrDefault();
 result.Add($"@{nameAttr.Name}");
 }
 else
 {
 result.Add($"@{prop.Name}");
 }
 }
 }
 return result;
}
public static string BuildSelect<T>(IEnumerable<string> additionalConstraints = null)
{
 var type = typeof(T);
 var sb = new StringBuilder();
 sb.Append("SELECT * FROM ");
 var tableAttribute = type.GetCustomAttributes(typeof(TableAttribute), true).FirstOrDefault();
 if (tableAttribute != null)
 {
 sb.Append(((TableAttribute)tableAttribute).Name);
 }
 else
 {
 sb.Append(type.Name);
 }
 var appendedClause = false;
 var parameters = GetParameters<T>(additionalConstraints);
 if (parameters.Count > 0)
 {
 sb.Append(" WHERE ");
 foreach (var p in parameters)
 {
 if (appendedClause)
 {
 sb.Append(" AND ");
 }
 sb.Append($"{p.Substring(1)} = {p}");
 appendedClause = true;
 }
 }
 return sb.ToString();
}

Advantages? Your code doesn't need modified, at all, to add support for a new type. Simply add the type, define [Table(name)], [Key] and [Column(name)] if necessary.

Downsides? Calling it frequently can become expensive, it would not be difficult to build a cache to hold all the queries and select the relevant one from, so that if you call BuildSelect<Foo>() it would return the result from the cache.

Now the point we're at is great if you only need select queries, if you need more than that we hit a slight issue in that we have a lot of code to reproduce to build a new query. Now there is a fix for that.

public class QueryProperty
{
 public PropertyInfo Property { get; set; }
 public string Name { get; set; }
}
public class QueryData
{
 public string TableName { get; set; }
 public List<QueryProperty> KeyProperties { get; set; } = new List<QueryProperty>();
 public List<QueryProperty> NonKeyProperties { get; set; } = new List<QueryProperty>();
 public List<QueryProperty> AllProperties { get; set; } = new List<QueryProperty>();
 public string AsSelect(bool explicitColumns = true, bool includeKey = true)
 {
 var sb = new StringBuilder();
 sb.Append("SELECT ");
 if (explicitColumns)
 {
 sb.Append(string.Join(", ", AllProperties.Select(x => x.Name)));
 }
 else
 {
 sb.Append('*');
 }
 sb.Append(" FROM ").Append(TableName);
 if (includeKey)
 {
 sb.Append(" WHERE ");
 sb.Append(string.Join(" AND ", KeyProperties.Select(x => $"{x.Name} = @{x.Name}")));
 }
 return sb.ToString();
 }
 public string AsInsert(bool explicitColumns = true)
 {
 var sb = new StringBuilder();
 sb.Append("INSERT INTO ").Append(TableName).Append(' ');
 if (explicitColumns)
 {
 sb.Append('(');
 sb.Append(string.Join(", ", AllProperties.Select(x => x.Name)));
 sb.Append(')');
 }
 sb.Append(" VALUES (");
 sb.Append(string.Join(", ", AllProperties.Select(x => $"@{x.Name}")));
 sb.Append(')');
 return sb.ToString();
 }
 public string AsUpdate(bool explicitColumns = true, bool includeKey = true)
 {
 var sb = new StringBuilder();
 sb.Append("UPDATE ").Append(TableName).Append(' ');
 sb.Append("SET ");
 sb.Append(string.Join(", ", NonKeyProperties.Select(x => $"{x.Name} = @{x.Name}")));
 if (includeKey)
 {
 sb.Append(" WHERE ");
 sb.Append(string.Join(" AND ", KeyProperties.Select(x => $"{x.Name} = @{x.Name}")));
 }
 return sb.ToString();
 }
 public static QueryData Build<T>(IEnumerable<string> additionalConstraints = null)
 {
 var result = new QueryData();
 var type = typeof(T);
 if (type.IsDefined(typeof(TableAttribute)))
 {
 var attr = (TableAttribute)type.GetCustomAttribute(typeof(TableAttribute), true);
 result.TableName = attr.Name;
 }
 else
 {
 result.TableName = type.Name;
 }
 foreach (var prop in type.GetProperties())
 {
 if (prop.IsDefined(typeof(ColumnAttribute), true))
 {
 var nameAttr = (ColumnAttribute)prop.GetCustomAttributes(typeof(ColumnAttribute), true).FirstOrDefault();
 result.AllProperties.Add(new QueryProperty { Property = prop, Name = nameAttr.Name });
 }
 else
 {
 result.AllProperties.Add(new QueryProperty { Property = prop, Name = prop.Name });
 }
 }
 var keys = result.AllProperties.Where(x => x.Property.IsDefined(typeof(KeyAttribute), true));
 if (keys.Any())
 {
 foreach (var key in keys)
 {
 result.KeyProperties.Add(key);
 }
 }
 var nonKeys = result.AllProperties.Where(x => !x.Property.IsDefined(typeof(KeyAttribute), true));
 if (nonKeys.Any())
 {
 foreach (var nonKey in nonKeys)
 {
 result.NonKeyProperties.Add(nonKey);
 }
 }
 if (additionalConstraints?.Any() == true)
 {
 foreach (var keyName in additionalConstraints)
 {
 var key = result.AllProperties.Where(x => x.Property.Name == keyName).FirstOrDefault();
 if (key == null)
 {
 throw new ArgumentException($"The value '{key}' provided for '{nameof(key)}' was not a property of the type '{type.FullName}'.");
 }
 result.KeyProperties.Add(key);
 }
 }
 return result;
 }
}

Now the call becomes:

var queryData = QueryData.Build<Foo>(new string[] { nameof(Foo.Name) });
var selectQuery = queryData.AsSelect();

I leave building a DELETE query up to you, once you have it I recommend posting a new Code Review question. :)

• t3chb0t: You've built Dapper-lite :-)
• Bassie: Nice, thanks for this great answer! I am already using dapper in my program when actually interacting with the db (but not for insert - for some reason I coudn't get this to work). My only question is, isn't this much use of reflection going to effect performance? And if so, as I'm already using Dapper (which I'm assuming also uses a lot of reflection), I wonder how much this would slow my application down. Thanks again!
• t3chb0t: @Bassie Dapper uses reflection, that's true, but it caches the results (per query) so next time there is no overhead, see the author's answer. Why are you concerned about performance? How many inserts/selects do you have?
• Bassie: I really don't have very many at all, but if you check my recent SO/serverfault questions I am expereiencing a really mysterious issue where db queries to the server via ASP MVC are very slow, but fast when running via console app, or asp mvc hosted on another pc on the same network. I have not been able to find the cause of this so now I'm worried about adding anything which is going to make the problem even worse
• Bassie: @t3chb0t stackoverflow.com/questions/44460339/… Link to the question if you were interested