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.
Best Answer
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:
.Close()
for each of their connections, but they will forget that if the.Close()
is not correctly placed in afinally
block, an exception could cause this call to never be executed. You must ensure your.Close()
calls are placed within afinally
block, and the easiest way to do this in .Net is with ausing
block.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.SqlDbType
enumeration, and assign a .NetDateTime
type to the parameter value. String dates should never enter into it. This leads nicely to the next item...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 theSqlParameterCollection.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.int
,string
, andDateTime
, 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.