Extending database factory class to use auditing class

I'm the the process of adding change auditing to my existing DB class which encapsulates database calls.

Previously I would just pass in a String and a SqlParameter array but now I've added the class 'DBData' to contain the values required for the auditing.

I'm not sure whether passing in this DBData object and then calling AuditAction() is the cleanest way to organise the code though and would appreciate input.

```
public class DBData
{
public DBData() { }
public DBData(object newValue, object oldValue)
{
this.NewValue = newValue;
this.OldValue = oldValue;
}

public int UserKey { get; set; }
public object NewValue { get; set; }
public object OldValue { get; set; }

public string Query { get; set; }
public SqlParameter[] Parameters { get; set; }
}

public static class DB
{
///
/// Connection String to Local Machines Database
///
private static readonly string connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
private static readonly DbProviderFactory factory = DbProviderFactories.GetFactory("System.Data.SqlClient");

public static void Update(DBData data)
{
try
{
AuditAction(data.UserKey, data.NewValue, data.OldValue);
InsertData(data.Query, data.Parameters);
}
catch (Exception)
{

throw;
}
}

///
/// Inserts data into the database
///
/// query
/// declared parameters
public static void InsertData(string sql, SqlParameter[] parameters)
{
try
{
using (DbConnection connection = factory.CreateConnection())
{
connection.ConnectionString = connectionString;

using (DbCommand command = factory.CreateCommand())
{
command.Connection = connection;
command.CommandType = CommandType.Text;

-
I don't see any reason to wrap code in an exception and just re-throw it (that is essentially like not having the wrapper in the first place).
Also I believe that just clutters the code so I would consider removing the exception wrapper altogether.

-
The public Update method just delegates to the Insert method. That seems a bit counter intuitive to me. Update to my mind would perform an
operation on an existing object, while insert adds a new one. I would consider renaming Insert to Update and have it as a method overload.

-
On second thoughts the Insert isn't really an insert at all as it will depend on the Sql supplied. It would consider calling that ExecuteNonQuery
to better indicate it's intent.

-
As @Malachi pointed out, I'm not sure of the factory implementation. To better abstract your db from the core DbProvider factories I might
consider creating my own interface and hiding everything behind that. This would allow your DbRepository to have no concept of where/how connections
are created either by using internal framework facilities or some other mechanism.

-
Does the class really need to be static? That limits what you can do in my opinion. I might consider writing an extension method to
create a db if you don't want to continue new'ing it everywhere. Otherwise you could consider injecting the DbRepository class into the methods
you need and only do it's construct in one place?

Here is an alternative code solution:

```
class DBData
{
public DBData() { }
public DBData(object newValue, object oldValue)
{
this.NewValue = newValue;
this.OldValue = oldValue;
}

public int UserKey { get; set; }
public object NewValue { get; set; }
public object OldValue { get; set; }

public string Query { get; set; }
public SqlParameter[] Parameters { get; set; }
}

interface IDbRepository
{
void Update(DBData data);
}

class DbRepository : IDbRepository
{
private readonly IDbFactory _dbFactory;

public DbContext(IDbFactory dbFactory)
{
_dbFactory = dbFactory;
}

public void ExecuteNonQuery(DBData data)
{
Audit(data.UserKey, data.NewValue, data.OldValue);
ExecuteNonQuery(data.Query, data.Parameters);
}

private void ExecuteNonQuery(string sql, SqlParameter[] parameters)
{
using (DbConnection connection = _dbFactory.CreateConnection())
{
connection.Open();

using (DbCommand command = _dbFactory.CreateCommand(connection, parameters))
{
command.ExecuteNonQuery();
}
}
}

///
/// Compare two objects and log the properties which have changed to the database.
///
/// id of logged in user
/// object with changes
/// copy of object before changes
public void Audit(int userKey, object newObject, object oldObject)
{
ChangeLogger logger = new ChangeLogger(userKey, newObject, oldObject);

// The name of the object should give a good indication of what the change was from
// E.g. ScanSchedule is obviously a change to the scan schedule
string className = newObject.GetType().Name;

logger.Audit();
if (!logger.Success)
{
throw logger.Exception;
}

logger.Changes.ForEach(log => Log(log.ObjectId, log.ValueNew, log.ValueOld, log.Property, className, DateTime.Now));
}

// I might consider moving this method into it's own Logger class
private void Log(long userKey, string valueNew, string valueOld, string property, string className, DateTime changeDate)
{
string insertLog
= "INSERT INTO ChangeLog "
+ "(UserKey, ObjectName, Property, NewValue, PreviousValue, ChangeDate) "
+ "VALUES "
+ "(@UserKey, @ObjectName, @Property, @NewValue, @PreviousValue, @ChangeDate)";

Insert(insertLog,
new[]
{
new SqlParameter("@UserKey", userKey),
new SqlParameter("@ObjectName", className),
new SqlParameter("@Property", property),
new SqlParameter("@NewValue", valueNew),
new SqlParameter("@PreviousValue", valueOld),
new SqlParameter("@ChangeDate", changeDate),
});
}
}

static class DbRepositoryProvider
{
private static readonly string connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;

public static IDbRepository GetRepository()
{
var factory = new DbFactory(connectionString);

return new DbRepository(factory);
}
}

class DbFactory : IDbFactory
{
private readonly string _connectionString;
private static readonly DbProviderFactory factory = DbProviderFactories.GetFactory("System.Data.SqlClient");

public DbFactory(string connectionString)
{
_connectionString = connectionString;
}

public DbConnectio

StackExchange Code Review Q#54638, answer score: 4