Skip to main content
Tweeted twitter.com/#!/StackCodeReview/status/649658543507947520
edited title
Link
JBond
  • 315
  • 2
  • 10

Refactoring CRUD commands to reduce boiler-plate codeSQL database

added 22 characters in body
Source Link
Heslacher
  • 51k
  • 5
  • 83
  • 177
  1. Create an empty instance of the response object

    Create an empty instance of the response object
  2. Create the SqlParameters (Note the SQL parameter names are always the same as a request object property names. The data types are also identical for primitive types, anything else is serialized and passed as an XML parameter).

    Create the SqlParameters (Note the SQL parameter names are always the same as a request object property names. The data types are also identical for primitive types, anything else is serialized and passed as an XML parameter).
  3. Create the DatabaseCommandInfo

    Create the DatabaseCommandInfo
  4. Call a DatabaseHelper method and return a result (could be scalar object, DataSet/DataTable)

    Call a DatabaseHelper method and return a result (could be scalar object, DataSet/DataTable)
  5. Populate the response object with the result from the database helper.

    Populate the response object with the result from the database helper.
  6. If a SqlException is thrown, store the error code and return the response with that code.

    public class ReadAsset { private const string StoredProc = "up_Assets_ReadAsset";

     private readonly IDatabaseHelper databaseHelper;
    
     public ReadAsset()
     {
       databaseHelper = new DatabaseHelper("Data Source=.; Initial Catalog=Assets; integrated security=true;");
     }
    
     /// <summary>
     /// Constructor used to inject dependencies
     /// </summary>
     /// <param name="databaseHelper"></param>
     public ReadAsset(IDatabaseHelper databaseHelper)
     {
       this.databaseHelper = databaseHelper;
     }
    
     public ReadAssetResponse Execute(ReadAssetRequest request)
     {
       var response = new ReadAssetResponse();
    
       var sqlParams = new[]
       {
         new SqlParameter("@TypeId", request.TypeId),
         new SqlParameter("@OwnershipId", request.OwnershipId),
         new SqlParameter("@GroupId", request.GroupId),
         new SqlParameter("@StatusIds", request.StatusIds.ToXml()),
       };
    
       var dbCommandInfo = new DatabaseCommandInfo(StoredProc, sqlParams, new[] {"AssetInfo"});
    
       try
       {
         var dataTable = databaseHelper.GetDataTable(dbCommandInfo);
    
         response.AssetInformation = new AssetInformation();
    
         if (DataTableIsNotPopulated(dataTable))
           return response;
    
         var row = dataTable.Rows[0];
         response.AssetInformation.Id = row.GetValue<int>("Id");
         response.AssetInformation.Address = row.GetValue<string>("Address");
         response.AssetInformation.Uprn = row.GetValue<string>("Uprn");
         response.AssetInformation.OSLocation = row.GetNullableValue<int>("OSLocation");
       }
       catch (SqlException sqlException)
       {
         response.Errors = new List<int> {sqlException.ErrorCode};
       }
    
       return response;
    
     }
    
     private static bool DataTableIsNotPopulated(DataTable dataTable)
     {
       return dataTable == null || dataTable.Rows == null || dataTable.Rows.Count != 1;
     }
    

    }

    If a SqlException is thrown, store the error code and return the response with that code.

I haven't included the database information (i.e. table/procs, etc) as it is not relevant or required here.

public class ReadAsset{private const string StoredProc = "up_Assets_ReadAsset";private readonly IDatabaseHelper databaseHelper;public ReadAsset(){databaseHelper = new DatabaseHelper("Data Source=.; Initial Catalog=Assets; integrated security=true;");}////// Constructor used to inject dependencies//////public ReadAsset(IDatabaseHelper databaseHelper){this.databaseHelper = databaseHelper;}public ReadAssetResponse Execute(ReadAssetRequest request){var response = new ReadAssetResponse();var sqlParams = new[]{new SqlParameter("@TypeId", request.TypeId),new SqlParameter("@OwnershipId", request.OwnershipId),new SqlParameter("@GroupId", request.GroupId),new SqlParameter("@StatusIds", request.StatusIds.ToXml()),};var dbCommandInfo = new DatabaseCommandInfo(StoredProc, sqlParams, new[]{"AssetInfo"});try{var dataTable = databaseHelper.GetDataTable(dbCommandInfo);response.AssetInformation = new AssetInformation();if(DataTableIsNotPopulated(dataTable))return response;var row = dataTable.Rows[0];response.AssetInformation.Id = row.GetValue("Id");response.AssetInformation.Address = row.GetValue("Address");response.AssetInformation.Uprn = row.GetValue("Uprn");response.AssetInformation.OSLocation = row.GetNullableValue("OSLocation");}catch(SqlException sqlException){response.Errors = new List{sqlException.ErrorCode};}return response;}private static bool DataTableIsNotPopulated(DataTable dataTable){return dataTable == null || dataTable.Rows == null || dataTable.Rows.Count!= 1;}}I haven't included the database information(i.e. table/procs, etc) as it is not relevant or required here.
  1. Create an empty instance of the response object

  2. Create the SqlParameters (Note the SQL parameter names are always the same as a request object property names. The data types are also identical for primitive types, anything else is serialized and passed as an XML parameter).

  3. Create the DatabaseCommandInfo

  4. Call a DatabaseHelper method and return a result (could be scalar object, DataSet/DataTable)

  5. Populate the response object with the result from the database helper.

  6. If a SqlException is thrown, store the error code and return the response with that code.

    public class ReadAsset { private const string StoredProc = "up_Assets_ReadAsset";

     private readonly IDatabaseHelper databaseHelper;
    
     public ReadAsset()
     {
       databaseHelper = new DatabaseHelper("Data Source=.; Initial Catalog=Assets; integrated security=true;");
     }
    
     /// <summary>
     /// Constructor used to inject dependencies
     /// </summary>
     /// <param name="databaseHelper"></param>
     public ReadAsset(IDatabaseHelper databaseHelper)
     {
       this.databaseHelper = databaseHelper;
     }
    
     public ReadAssetResponse Execute(ReadAssetRequest request)
     {
       var response = new ReadAssetResponse();
    
       var sqlParams = new[]
       {
         new SqlParameter("@TypeId", request.TypeId),
         new SqlParameter("@OwnershipId", request.OwnershipId),
         new SqlParameter("@GroupId", request.GroupId),
         new SqlParameter("@StatusIds", request.StatusIds.ToXml()),
       };
    
       var dbCommandInfo = new DatabaseCommandInfo(StoredProc, sqlParams, new[] {"AssetInfo"});
    
       try
       {
         var dataTable = databaseHelper.GetDataTable(dbCommandInfo);
    
         response.AssetInformation = new AssetInformation();
    
         if (DataTableIsNotPopulated(dataTable))
           return response;
    
         var row = dataTable.Rows[0];
         response.AssetInformation.Id = row.GetValue<int>("Id");
         response.AssetInformation.Address = row.GetValue<string>("Address");
         response.AssetInformation.Uprn = row.GetValue<string>("Uprn");
         response.AssetInformation.OSLocation = row.GetNullableValue<int>("OSLocation");
       }
       catch (SqlException sqlException)
       {
         response.Errors = new List<int> {sqlException.ErrorCode};
       }
    
       return response;
    
     }
    
     private static bool DataTableIsNotPopulated(DataTable dataTable)
     {
       return dataTable == null || dataTable.Rows == null || dataTable.Rows.Count != 1;
     }
    

    }

I haven't included the database information (i.e. table/procs, etc) as it is not relevant or required here.

  1. Create an empty instance of the response object
  2. Create the SqlParameters (Note the SQL parameter names are always the same as a request object property names. The data types are also identical for primitive types, anything else is serialized and passed as an XML parameter).
  3. Create the DatabaseCommandInfo
  4. Call a DatabaseHelper method and return a result (could be scalar object, DataSet/DataTable)
  5. Populate the response object with the result from the database helper.
  6. If a SqlException is thrown, store the error code and return the response with that code.
public class ReadAsset{private const string StoredProc = "up_Assets_ReadAsset";private readonly IDatabaseHelper databaseHelper;public ReadAsset(){databaseHelper = new DatabaseHelper("Data Source=.; Initial Catalog=Assets; integrated security=true;");}////// Constructor used to inject dependencies//////public ReadAsset(IDatabaseHelper databaseHelper){this.databaseHelper = databaseHelper;}public ReadAssetResponse Execute(ReadAssetRequest request){var response = new ReadAssetResponse();var sqlParams = new[]{new SqlParameter("@TypeId", request.TypeId),new SqlParameter("@OwnershipId", request.OwnershipId),new SqlParameter("@GroupId", request.GroupId),new SqlParameter("@StatusIds", request.StatusIds.ToXml()),};var dbCommandInfo = new DatabaseCommandInfo(StoredProc, sqlParams, new[]{"AssetInfo"});try{var dataTable = databaseHelper.GetDataTable(dbCommandInfo);response.AssetInformation = new AssetInformation();if(DataTableIsNotPopulated(dataTable))return response;var row = dataTable.Rows[0];response.AssetInformation.Id = row.GetValue("Id");response.AssetInformation.Address = row.GetValue("Address");response.AssetInformation.Uprn = row.GetValue("Uprn");response.AssetInformation.OSLocation = row.GetNullableValue("OSLocation");}catch(SqlException sqlException){response.Errors = new List{sqlException.ErrorCode};}return response;}private static bool DataTableIsNotPopulated(DataTable dataTable){return dataTable == null || dataTable.Rows == null || dataTable.Rows.Count!= 1;}}I haven't included the database information(i.e. table/procs, etc) as it is not relevant or required here.
Source Link
JBond
  • 315
  • 2
  • 10

Refactoring to reduce boiler-plate code

I have a large application that focuses on using dependency injection. I've made a small (as I possibly could) piece of sample code to make this question more manageable. Essentially, I have a lot of boiler-plate code occurring for a number of commands that call stored procedures and return a response object back to the caller. I'd really like to find a more generic (if possible) way of doing this.

Normally, all of this code sits inside a Web Api and would have controllers executing the commands.

The complete code example is as follows (NOTE The code to refactor is at the very bottom, the rest is just supporting code):

Request/Response objects

Request

All requests inherit from BaseRequest which just contains the identifier for the api performing the request (this is verified within the proc):

 public class BaseRequest
  {
    public string Identifier { get; set; }
  }

Here is an example of a request class for a command:

public class ReadAssetRequest : BaseRequest
  {
    public int TypeId { get; set; }
    public int OwnershipId { get; set; }
    public int GroupId { get; set; }
    public IEnumerable<int> StatusIds { get; set; } 
  }

Response

All responses inherit from BaseResponse which just contains a list of errors from the stored procedures (if any):

public class BaseResponse
  {
    public List<int> Errors { get; set; }
  }

Here is an example of a response class for a command:

public class ReadAssetResponse : BaseResponse
  {
    public AssetInformation AssetInformation { get; set; }
  }

This is the class for the object being returned:

public class AssetInformation
  {
    public int Id { get; set; }

    public string Uprn { get; set; }

    public string Address { get; set; }

    public int? OSLocation { get; set; }
  }

To talk to the database. There is a database helper:

DatabaseHelper Interface

public interface IDatabaseHelper
  {
    void ExecuteNonQuery(DatabaseCommandInfo data);
    DataSet GetDataSet(DatabaseCommandInfo data);
    DataTable GetDataTable(DatabaseCommandInfo data);
  }

DatabaseHelper Class

public class DatabaseHelper : IDatabaseHelper
  {
    private readonly string connectionString;

    public DatabaseHelper(string connectionString)
    {
      this.connectionString = connectionString;
    }

    public DataSet GetDataSet(DatabaseCommandInfo data)
    {
      var ds = new DataSet();
      using (var con = new SqlConnection(connectionString))
      {
        con.Open();
        using (var cmd = GetSqlCommand(data, con))
        {
          using (var rdr = cmd.ExecuteReader())
          {
            ds.Load(rdr, data.Option, data.TableNames);
          }
          cmd.Parameters.Clear();
        }
      }
      return ds;
    }

    public DataTable GetDataTable(DatabaseCommandInfo data)
    {
      var dt = new DataTable();
      using (var con = new SqlConnection(connectionString))
      {
        con.Open();
        using (var cmd = GetSqlCommand(data, con))
        {
          using (var rdr = cmd.ExecuteReader())
          {
            dt.Load(rdr);
          }
          cmd.Parameters.Clear();
        }
      }
      return dt;
    }

    public void ExecuteNonQuery(DatabaseCommandInfo data)
    {
      using (var con = new SqlConnection(connectionString))
      {
        con.Open();
        using (var cmd = new SqlCommand(data.StoredProcName, con))
        {
          cmd.CommandType = data.CommandType;
          cmd.Parameters.AddRange(data.Parameters);
          cmd.ExecuteNonQuery();
          cmd.Parameters.Clear();
        }
      }
    }

    private SqlCommand GetSqlCommand(DatabaseCommandInfo data, SqlConnection sqlConnection)
    {
      var cmd = new SqlCommand(data.StoredProcName, sqlConnection)
      {
        CommandType = data.CommandType
      };

      if(data.Parameters != null)
        cmd.Parameters.AddRange(data.Parameters);

      return cmd;
    }

  }

The database helper takes a DatabaseCommandInfo object so it knows what stored proc to call and with what SqlParameters:

DatabaseCommandInfo class

 public class DatabaseCommandInfo
  {
    public string StoredProcName { get; private set; }
    public SqlParameter[] Parameters { get; private set; }
    public string[] TableNames { get; private set; }
    public LoadOption Option { get; private set; }
    public CommandType CommandType { get; set; }

    public DatabaseCommandInfo(string storeProcName, SqlParameter[] spParams)
    {
      StoredProcName = storeProcName;
      Parameters = spParams;
      CommandType = CommandType.StoredProcedure;
    }


    public DatabaseCommandInfo(string storeProcName, SqlParameter[] spParams, string[] tableNames)
    {
      StoredProcName = storeProcName;
      Parameters = spParams;
      TableNames = tableNames;
      Option = LoadOption.OverwriteChanges;
      CommandType = CommandType.StoredProcedure;
    }
  }

Helper/Extension Methods

The command uses some Helper/Extension methods.

DataRowExtensions

Extension methods used to help retrieval of values from a DataRow:

public static class DataRowExtension
  {
    public static T GetValue<T>(this DataRow row, string columnName)
    {
      if (row != null && row.Table.Columns.Count > 0 && row[columnName] != DBNull.Value)
      {
        return (T)Convert.ChangeType(row[columnName], typeof(T));
      }
      return default(T);
    }

    public static T? GetNullableValue<T>(this DataRow row, string columnName) where T : struct
    {
      if (DBNull.Value.Equals(row[columnName]))
      {
        return null;
      }
      return (T)Convert.ChangeType(row[columnName], typeof(T));
    }
  }

Helper method to serialize object as XML

Used when a SQL parameter is not a primitive type, the value is passed as XML to the proc.

public static class ListExtensions
  {
    public static string IdsToXml(this IEnumerable<int> ids)
    {
      var idList = ids.ToList();
      if (!idList.Any())
        return new XElement("Ids").ToString();

      var xmlElements = new XElement("Ids", idList.Select(i => new XElement("x", new XAttribute("i", i))));
      return xmlElements.ToString();
    }

    public static string ToXml<T>(this T items)
    {
      return Serializer.SerializeObject(items);
    }
  }

Serializer class

public static class Serializer
  {
    public static string SerializeObject<T>(T toSerialize)
    {
      var xmlSerializer = new XmlSerializer(toSerialize.GetType());

      using (var textWriter = new StringWriter())
      {
        xmlSerializer.Serialize(textWriter, toSerialize);
        return textWriter.ToString();
      }
    }
  }

Actual command to refactor

With all the above supporting code. The following is an example of a simple command that contains boiler-plate code I'd like to refactor. The flow of the commands are:

  1. Create an empty instance of the response object

  2. Create the SqlParameters (Note the SQL parameter names are always the same as a request object property names. The data types are also identical for primitive types, anything else is serialized and passed as an XML parameter).

  3. Create the DatabaseCommandInfo

  4. Call a DatabaseHelper method and return a result (could be scalar object, DataSet/DataTable)

  5. Populate the response object with the result from the database helper.

  6. If a SqlException is thrown, store the error code and return the response with that code.

    public class ReadAsset { private const string StoredProc = "up_Assets_ReadAsset";

     private readonly IDatabaseHelper databaseHelper;
    
     public ReadAsset()
     {
       databaseHelper = new DatabaseHelper("Data Source=.; Initial Catalog=Assets; integrated security=true;");
     }
    
     /// <summary>
     /// Constructor used to inject dependencies
     /// </summary>
     /// <param name="databaseHelper"></param>
     public ReadAsset(IDatabaseHelper databaseHelper)
     {
       this.databaseHelper = databaseHelper;
     }
    
     public ReadAssetResponse Execute(ReadAssetRequest request)
     {
       var response = new ReadAssetResponse();
    
       var sqlParams = new[]
       {
         new SqlParameter("@TypeId", request.TypeId),
         new SqlParameter("@OwnershipId", request.OwnershipId),
         new SqlParameter("@GroupId", request.GroupId),
         new SqlParameter("@StatusIds", request.StatusIds.ToXml()),
       };
    
       var dbCommandInfo = new DatabaseCommandInfo(StoredProc, sqlParams, new[] {"AssetInfo"});
    
       try
       {
         var dataTable = databaseHelper.GetDataTable(dbCommandInfo);
    
         response.AssetInformation = new AssetInformation();
    
         if (DataTableIsNotPopulated(dataTable))
           return response;
    
         var row = dataTable.Rows[0];
         response.AssetInformation.Id = row.GetValue<int>("Id");
         response.AssetInformation.Address = row.GetValue<string>("Address");
         response.AssetInformation.Uprn = row.GetValue<string>("Uprn");
         response.AssetInformation.OSLocation = row.GetNullableValue<int>("OSLocation");
       }
       catch (SqlException sqlException)
       {
         response.Errors = new List<int> {sqlException.ErrorCode};
       }
    
       return response;
    
     }
    
     private static bool DataTableIsNotPopulated(DataTable dataTable)
     {
       return dataTable == null || dataTable.Rows == null || dataTable.Rows.Count != 1;
     }
    

    }

I haven't included the database information (i.e. table/procs, etc) as it is not relevant or required here.