patterncsharpMinor
Database Update class using SQLCommand
Viewed 0 times
updatesqlcommanddatabaseusingclass
Problem
I would like to have advice on this use of base class and derived class. This all works but is it a good way to achieve a SOLID class?
```
public class ProductRepository : GenericDataExport
{
private const string DB_TABLE = "[dbo].[ReplenEdit]";
private int rowsUpdated;
private int primaryKey;
private string columnName;
private int replenishAmount;
public void SetUpdateReplenAmount(int primaryKey, int replenishAmount)
{
this.primaryKey = primaryKey;
this.columnName = "ReplenishAmount";
this.replenishAmount = replenishAmount;
}
/// Method to check if database has been updated
/// Checks to see if the the Rows Updated field is greather than 0
/// This indicates that the database has had a least one row updated.
///
/// True or False
public bool HasDatabaseBeenUpdated()
{
return UpdateTableHelper.HasDatabaseBeenUpdated(this.rowsUpdated);
}
/// Create Update SQL statement and then update table using SQL Command
///
/// SQLConnection
protected override void ActionUpdateCommands(SqlConnection conn)
{
string sql = CreateSQLStatement();
this.rowsUpdated = 0;
UpdateTable(conn, sql);
}
#region Class Implementation
/// Method to update Database table with Replenish Amount
///
/// SQLConnection object
/// SQL Statement
private void UpdateTable(SqlConnection conn, string sql)
{
using (SqlCommand command = new SqlCommand(sql, conn))
{
comm
```
public class ProductRepository : GenericDataExport
{
private const string DB_TABLE = "[dbo].[ReplenEdit]";
private int rowsUpdated;
private int primaryKey;
private string columnName;
private int replenishAmount;
public void SetUpdateReplenAmount(int primaryKey, int replenishAmount)
{
this.primaryKey = primaryKey;
this.columnName = "ReplenishAmount";
this.replenishAmount = replenishAmount;
}
/// Method to check if database has been updated
/// Checks to see if the the Rows Updated field is greather than 0
/// This indicates that the database has had a least one row updated.
///
/// True or False
public bool HasDatabaseBeenUpdated()
{
return UpdateTableHelper.HasDatabaseBeenUpdated(this.rowsUpdated);
}
/// Create Update SQL statement and then update table using SQL Command
///
/// SQLConnection
protected override void ActionUpdateCommands(SqlConnection conn)
{
string sql = CreateSQLStatement();
this.rowsUpdated = 0;
UpdateTable(conn, sql);
}
#region Class Implementation
/// Method to update Database table with Replenish Amount
///
/// SQLConnection object
/// SQL Statement
private void UpdateTable(SqlConnection conn, string sql)
{
using (SqlCommand command = new SqlCommand(sql, conn))
{
comm
Solution
ProductRepository is 60 lines, you don't need a region.Don't abbreviate when you don't need to: there's no need to shorten
ReplenishAmount to Replen in SetUpdateReplenAmount.In UpdateTable you spend two lines on doing something that can be done in one:
command.Parameters.Add("@replenishAmt", SqlDbType.Int).Value = this.replenishAmount;But IMHO it's even worse that the parameter name --
"@replenishAmt" -- is repeated twice here and is also defined in CreateSQLStatement, when it clearly should be a const at the class level.Moreover, why isn't
columnName a const either?But more importantly: all of your code is solving a problem that has already been solved by ORMs like NHibernate or Entity Framework. You have written a 60 line class that can do one single thing: update the field
ReplenishAmount in the table ReplenEdit.My advice: throw all of this away, implement a decent ORM and focus on the actual business logic.
Code Snippets
command.Parameters.Add("@replenishAmt", SqlDbType.Int).Value = this.replenishAmount;Context
StackExchange Code Review Q#92446, answer score: 3
Revisions (0)
No revisions yet.