C# Design for SQL connection and commands

Question

Currently I'm working on system that works with database, and I would like to have it done elegant way. So I have abstracted DBConnection into one class, DBCommands into another class. (DBCommands : DBConnection)

There is also a class SQLUsage in which I'm creating new thread that works in a loop, it checks all the time if there's any object in BlockingCollection to take care of, if there is that thread is processing it, and it uses DBConnection and DBCommands for it. I am having a problem accesing DBCommands from SQLUsage, since the DBCommands object is created in DBConnection (I have to pass MySqlConnection object into DBCommands)

Is that good solution? Or I should change the design, I would like to use those classes in another app as well (part of same, bigger system).

I'll tell you bit more about this app. It is a server app that handles connections from multiple clients. Each client sends some info to server, that is stored in BlockingCollection<thatinfo>. Another thread is using items from BlockingCollection<thatinfo> to communicate with mysql server, and insert some data to it, according to objects<thatinfo>.

And I'm having problem with design. So far I have a main class, class for handling TCP/IP connections, thatinfo class, SQLUsage, DBConnection and DBCommands class.

Since BlockingCollection is initiated in main class and it is passed to both TCP/IP handling class and SQL Handling class it is not a problem. My problem is to properly design SQLHandling classes. I want it to be done in elegant way.

So I figured out it would be good to abstract connection and commands from the class where I process the info. So I already have working SQLConnection class, I had some commands in it as well and it worked. But next step was to abstract commands in another class, since there will be many overloaded methods (for example Insert with 1 arg, Insert with 2 args etc.)

So my question is not about the code, I know how to create it, it's more about design. If abstracting sql commands in another class was good idea or not? Should both DBConnection and DBCommands inherit from SQLHandling class? or only DBConnection : DBCommands ?

And another question, probably easier to answer, is there more elegant way to do sql commands than just overloading methods? Like 1 universal INSERT method that handles all arguments?

For now having few overloaded insert methods seems to be a good idea since in this app I'm working on, there will be only 2 different kinds of inserts (just different amount of arguments), but in another app I wanted use the same classes I could really use universal Insert method.

Answer 1

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.