HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Database Update class using SQLCommand

Submitted by: @import:stackexchange-codereview··
0
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

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.