patterncsharpMinor
Database access class
Viewed 0 times
databaseclassaccess
Problem
I'm looking for any comments or feedback on my database access class. Security and speed are two things I'm most concerned about.
One thing to note is this class has to work in a C# .NET 2 environment, so anything that's more modern would be interesting to me, but please note in the title of your answer if the comments/feedback require a newer .NET version.
```
using Microsoft.Practices.EnterpriseLibrary.Common;
using Microsoft.Practices.EnterpriseLibrary.Data;
using System.Data;
using System.Data.SqlClient;
using System.Data.Common;
///
/// This is the base class for database access classes. This is the only
/// class that should directly talk to the database. Every class or page
/// that neads to access the database should be refering to this or a
/// derived class.
///
public class DatabaseAccess
{
static string LastDatabaseName = "";
static Database database = null;
static int errorCount = 0;
///
/// Execute a SQL statement on the default database
///
/// The SQL statement to execute
/// DataTable of selected results
public static DataTable ExecSQL(string SQL)
{
List Parameters = new List();
return ExecSQL("", SQL, Parameters);
}
///
/// Execute a SQL statement on the default database
///
/// The SQL statement to execute
/// The parameters for the SQL statement
/// DataTable of selected results
public static DataTable ExecSQL(string SQL, List Parameters)
{
return ExecSQL("", SQL, Parameters);
}
///
/// Execute a SQL statement on the requested database
///
/// The database to execute the SQL on
/// The SQL statement to execute
/// DataTable of selected results
public static DataTable ExecSQL(string DatabaseName, string SQL)
{
List Parameters = new List();
return ExecSQL(DatabaseName, SQL, Parameters);
}
///
/// Execute a SQL statement on the requested database
///
/// The database
One thing to note is this class has to work in a C# .NET 2 environment, so anything that's more modern would be interesting to me, but please note in the title of your answer if the comments/feedback require a newer .NET version.
```
using Microsoft.Practices.EnterpriseLibrary.Common;
using Microsoft.Practices.EnterpriseLibrary.Data;
using System.Data;
using System.Data.SqlClient;
using System.Data.Common;
///
/// This is the base class for database access classes. This is the only
/// class that should directly talk to the database. Every class or page
/// that neads to access the database should be refering to this or a
/// derived class.
///
public class DatabaseAccess
{
static string LastDatabaseName = "";
static Database database = null;
static int errorCount = 0;
///
/// Execute a SQL statement on the default database
///
/// The SQL statement to execute
/// DataTable of selected results
public static DataTable ExecSQL(string SQL)
{
List Parameters = new List();
return ExecSQL("", SQL, Parameters);
}
///
/// Execute a SQL statement on the default database
///
/// The SQL statement to execute
/// The parameters for the SQL statement
/// DataTable of selected results
public static DataTable ExecSQL(string SQL, List Parameters)
{
return ExecSQL("", SQL, Parameters);
}
///
/// Execute a SQL statement on the requested database
///
/// The database to execute the SQL on
/// The SQL statement to execute
/// DataTable of selected results
public static DataTable ExecSQL(string DatabaseName, string SQL)
{
List Parameters = new List();
return ExecSQL(DatabaseName, SQL, Parameters);
}
///
/// Execute a SQL statement on the requested database
///
/// The database
Solution
A few quick ideas:
If you use c# 3.0 or later
- Check spelling.
- Use
string.Emptyinstead of "" for improved readability and performance.
- Always use visibility modifiers - For example your fields lack the typical "private" keyword. Example
private static Database database = null;
- Re-evaluate your design choice to go with a static class. Static classes are know for causing head-aches such as threading problems. Read more here to start with if you are unsure. Just removing all "static" keywords will make the class just as usable.
- Use lower case for local variables and parameters. For example: "var parameters = new List();"
- As for the error-counting-logic I don't even know where to start... :-/ Perhaps the whole thing can be done in some other way.
- Consider the naming of
ExecSQL- SQL commands can be both inserts and selects and also other types of commands, while this class concerns itself with select
- Consider using
IEnumerableinstead ofListsince you're only iterating theList.
If you use c# 3.0 or later
- Use the
varkeyword if target type is redundant. ExampleList Parameters = new List();
Context
StackExchange Code Review Q#2578, answer score: 6
Revisions (0)
No revisions yet.