C# Database Communication – Techniques for SQL

When naming classes it's often tempting to include words that describe usage in programming terms. These words might include abstract, entity, and/or base. So you end up creating classes like AbstractOffice or BaseOffice. The name of the class implies two meanings what it does Office and how it's used Abstract. I don't recommend doing this. It adds extra details in the name that it doesn't need.

KISS (Keep It Simple Stupid)

I recently refactored a project I did a year ago to use simple names, and it became much easier to read the code. Give it a try and see if it helps. You can always rename the classes later.

public class Employee : Data
{
    public int Id { get; set; }
    public string LastName { get; set; }
    public string FirstName { get; set; }
}

[Sql(@"SELECT [EmployeeId], [LastName], [FirstName] 
   FROM [Employees] 
   ORDER BY [LastName], [FirstName]")
public class Employees : Query
{
    public int Id { get; set; }
}

When you remove programming specific terms (i.e. I removed Select, Entity and renamed Sql to Query). The classes become more readable. Now their names infer only specific information (i.e. this is about an employee). The implementation that it uses SQL or represents an entity pattern is irrelevant. That information should go into the class comments not it's name.

In the future, you will have a bug. Which description is easier to understand, and a result easier to find in the source code.

Employee John Smith is not an active member of Employees.

or

EmployeeEntry John is not an active member of SelectEmployee

That's just an example, but later when you're using those classes the code becomes difficult to read. A sea of terms that aren't relevant to what the code is doing.

I don't see a really answerable question in there anywhere, but I can at least address some general concerns here. Over the years I've noticed a few common mistakes in this area. I'll list them below, in hopes of helping you avoid them:

1. Re-using the same SqlConnection object. It's counter-intuitive, but .Net is designed such that it's more effective in most cases to create a new DbConnection object for each call to the database. Of course there are exceptions, but if you find yourself wanting to keep re-using a single shared DbConnection, you'll have problems. It's worth noting that this is thanks to a connection pooling feature that is implemented by the provider, and that some providers may do this differently... however, most of the major providers do implement connection pooling, such that this does hold.
2. Failing to properly close your database connections. Since we're now creating a new DbConnection object for each query, it's important that we also close all these connections. Failure to do so can result in locking the database for new access... effectively creating a denial of service situation. Often, someone will remember to call .Close() for each of their connections, but they will forget that if the .Close() is not correctly placed in a finally block, an exception could cause this call to never be executed. You must ensure your .Close() calls are placed within a finally block, and the easiest way to do this in .Net is with a using block.
3. Methods that accept only sql command strings. Often there will be method somewhere that accepts an sql string, and returns a DataSet, DataTable, or even DbDataReader (fwiw, my preference is an IEnumerable<IDataRecord>). Perhaps this is paired with a similar method that updates the database. The problem with these methods is that, if they accept only an sql string as input, they practically force you to write code that will be horribly vulnerable to Sql Injection attacks. The idea of running all your sql through a common place is on the right track, but it's missing an important piece: these methods must also provide a way to accept sql parameter data that is separate from the sql command string. There are a number of ways to do this, so picking a particular best one is outside the scope of this post. The one piece of additional advice I can give is to avoid overloading the method to not require this if there really are no parameters for a query. That overload tends to get abused, resulting in security holes. IMO, it's better to force callers to explicitly pass some kind of construct with zero query parameters, and thus ensure at least some familiarity with the concept.
4. Formatting a date string for a call to a database. If you're trying to format a date for use in an sql query, you're doing something wrong. Date values in the database should use a datetime type for the column. Sql Server has several available, including datetime, date, and datetime2, and these types are typical of other databases. When sending date information to the database as part of a query, make sure to use a query parameter that is explicitly typed to the matching type in the SqlDbType enumeration, and assign a .Net DateTime type to the parameter value. String dates should never enter into it. This leads nicely to the next item...
5. Allowing .Net to try to guess your parameter types. I often see examples posted that demonstrate the SqlParameterCollection.AddWithValue() method, and I'm not a fan. When you use this method, .Net will try to infer the type of your parameter. Frankly, there are several situations where it's just not very good. Using strings as dates is one example. Choosing NVarChar when it should choose VarChar is another. Choosing the wrong length for a VarChar/NVarChar is a third. Any of these can be overcome, and most of the time the query will still work... but it can also fail in unexpected places, and worse, those failures will be in production, rather than during testing. Additionally, sometimes a type mismatch can result in per-row conversions in computing a results set, which can slow things down considerably. Or a type mismatch can result in the database failing to use an obvious important index for a query, making the query take orders of magnitudes longer than it should. Better to favor the SqlParameterCollection.Add() method that asks you to give it an explicit parameter type. Again, this is provider-specific, but most providers make these two methods available.
6. Public methods that ask for an sql command string. As I said earlier, forcing all database through a handful of methods is a good thing... but to make sure this is enforced, those methods need to be private. My data layers will typically have one method for data retrieval, one that only makes changes (insert, update, delete), and maybe one more that returns multiple result sets in the same call (SqlDataReader can do this, though it often isn't used). However, these methods should be private. What you want for the public-facing part of your data layer is one public method per query, and these many public methods then call into the few private methods to complete their tasks. Instead of asking for an sql command, the public methods ask for typed arguments that match the parameters that will be used with the query. These types should most-often be .Net basic types, like int, string, and DateTime, to avoid mixing data and business layers, though occasionally asking for a complicated business object may be expedient... not the purest design, but sometimes sullying the pure design can be okay in the name of less code, DRY, and YAGNI.