Design Pattern to Add, Edit and Delete Records

Question

This code allows Add, Edit and Delete a database record for the Category table. Separate Service classes are implemented to handle these operations which are called via the Web API Endpoint CategoriesController.

• I want to improve the current code pattern because I am not sure calling BaseBusinessService.Execute() on CategoriesController is good practise?

• Execute() is declared as Public in the Abstract Class BaseBusinessService. I am not sure whether this is a good idea?

The following code with basic explanation.

BaseController.cs - to abstract the common tasks, such as store the connection string.

public class BaseController
{
    protected string DbConnectionString { get; }

    protected BaseController(IConfiguration configuration)
    {
        DbConnectionString = configuration.GetSection("connectionStrings:databaseConnectionString").Value;
    }
}

CategoriesController.cs - called via the api endpoint.

[Route("api/v1/categories")]
public class CategoriesController : BaseController
{
    public CategoriesController(IConfiguration configuration) : base(configuration)
    {
    }

    [HttpPost]
    public IActionResult Post([FromBody]CategoryModel model)
    {
        new CategoryAddService(DbConnectionString, model).Execute();

        return new OkResult();
    }

    [HttpPut("{id}")]
    public IActionResult Put(int id, [FromBody]CategoryModel model)
    {
        new CategoryEditService(DbConnectionString, id, model).Execute();

        return new OkResult();
    }

    [HttpDelete("{id}")]
    public IActionResult Delete(int id)
    {
        new CategoryDeleteService(DbConnectionString, id).Execute();

        return new OkResult();
    }
}

BaseBusinessService.cs - to abstract common methods, the implemented service classes must inherit from this class.

public abstract class BaseBusinessService
{
    protected string DbConnectionString { get; }

    protected BaseBusinessService(string dbConnectionString)
    {
        DbConnectionString = dbConnectionString;
    }

    protected abstract void OnValidate();
    protected abstract void OnExecute();

    // This method is called from the Controller, that's why this is Public. Not a good idea?
    public void Execute()
    {
        OnValidate();
        OnExecute();
    }
}

CategoryAddService.cs - Gets called from the Api Endpoint and adds a single record into the database.

public class CategoryAddService : BaseBusinessService
{
    CategoryModel _categoryModel;

    public CategoryAddService(string dbConnectionString, CategoryModel categoryModel) : base(dbConnectionString)
    {
        _categoryModel = categoryModel;
    }

    protected override void OnValidate()
    {
    }

    protected override void OnExecute()
    {
        var poco = PreparePoco(_categoryModel.CategoryName);

        AddCategoryRecord(poco);
    }

    CategoryPoco PreparePoco(string categoryName)
    {
        return new CategoryPoco()
        {
            CategoryName = categoryName
        };
    }

    void AddCategoryRecord(CategoryPoco poco)
    {
        using (var connection = new SqlConnection(DbConnectionString))
        {
            connection.Insert(poco);
        }
    }
}

CategoryEditService.cs - Gets called from the api endoint, validates the existing record and updates the row in the database.

public class CategoryEditService : BaseBusinessService
{
    readonly string _categoryName;
    readonly int _id;

    public CategoryEditService(string dbConnectionString, int id, CategoryModel categoryModel) : base(dbConnectionString)
    {
        _categoryName = categoryModel.CategoryName;
        _id = id;
    }

    protected override void OnValidate()
    {
        // Validate and throw the error if the doesn't exists.
    }

    protected override void OnExecute()
    {
        var poco = PreparePoco(_categoryName);

        UpdateCategoryRecord(poco);
    }

    CategoryPoco PreparePoco(string categoryName)
    {
        return new CategoryPoco()
        {
            CategoryId = _id,
            CategoryName = categoryName
        };
    }

    void UpdateCategoryRecord(CategoryPoco poco)
    {
        using (var connection = new SqlConnection(DbConnectionString))
        {
            connection.Update(poco);
        }
    }
}

CategoryDeleteService.cs - Gets called via the api endoing, validates the existing row and deletes a record from the database.

public class CategoryDeleteService : BaseBusinessService
{
    readonly int _id;

    public CategoryDeleteService(string dbConnectionString, int id) : base(dbConnectionString)
    {
        _id = id;
    }

    protected override void OnValidate()
    {
        // Validate and throw the error if the doesn't exists.
    }

    protected override void OnExecute()
    {
        DeleteCategoryRecord();
    }

    void DeleteCategoryRecord()
    {
        using (var connection = new SqlConnection(DbConnectionString))
        {
            connection.Delete<CategoryPoco>(_id);
        }
    }
}

CategoryPoco.cs - This is used for the Dapper and SimpleCRUD ORM.

using Dapper;
namespace AppPattern.Categories.Services
{
    [Table("Category")]
    public sealed class CategoryPoco
    {
        [Key]
        public int CategoryId { get; set; }
        public string CategoryName { get; set; }
    }
}

CategoryModel.cs - A model which does the validation using FluentValidation library.

using FluentValidation;
namespace AppPattern.Categories.Models
{
    public class CategoryModel
    {
        string _categoryName;
        public string CategoryName
        {
            get { return _categoryName; }
            set { _categoryName = value.Trim(); }
        }
    }

    class CategoryAddModelValidator : AbstractValidator<CategoryModel>
    {
        public CategoryAddModelValidator()
        {
            RuleFor(x => x.CategoryName).NotEmpty();
            RuleFor(x => x.CategoryName).Length(1, 128)
                .When(x => !string.IsNullOrEmpty(x.CategoryName));
        }
    }
}

Answer 1

Dependency inversion principle violation

CategoriesController depend on Category[operation]Services (concrete implementations) rather depending on abstractions.

How to prevent this rule violation? Write unit tests.

Side note: BaseController depend on concrete configuration value. But that value is used only in data access. So data access classes should depend on value, not controller itself.

Pattern names should match implementations.

What I mean is, that Category[operation]Service-s are not services at all. They are commands. So maybe you mean CategoryEditCommand, CategoryDeleteCommand and so on.

I will give you a more concrete example:

public interface ICategoryFactory {
    CategoryModel GetCategory();
}

Everyone expect that Factory should have Create method, not Get, Set or something else.

What we expect for service is like:

public interface ICategoryService
{
   CategoryModel GetCategory(int id);
   void AddCategory(CategoryModel model);
   void EditCategory(int id, CategoryModel model);
   void DeleteCategory(int id);
}

Using Command pattern here is overhead. Command pattern is commonly used for hiding how the command is executed and to provide comprehensive way of adding extensions.

Suggestions

So for your scenario I suggest:

1. Refactor your services to one ICategoryService that I have suggested above.

2. Make CategoriesController depend on ICategoryService.

3. Remove configuration value dependecy from BaseController and add it to implementation of ICategoryService

   Don't forget to register ICategoryService in your IoC container.

Update 1: About Validators. For separating concepts I would suggest following hierarchy:

ICategoryStore - nothing business related, just db operations.

public interface ICategoryStore
{
   CategoryModel GetCategory(int id);
   void AddCategory(CategoryModel model);
   void EditCategory(int id, CategoryModel model);
   void DeleteCategory(int id);
}

Based on how complex you can decide how higher should be your validation abstraction, I can suggest following:

IValidator - Encapsulates validation logic for entity.

public interface ICategoryValidator 
{
   void ValidateAdd(ModelState state);
   void ValidateEdit(ModelState state);
   void ValidateDelete(ModelState state);
}

And finally ICategoryService which implementation should depend on ICategoryStore and ICategoryValidator.

With this approach you can abstract higher, by introducing IStore<T> and IValidator<T>.

Answer 2

• A shallow answer: BaseBusinessService can be a class interface instead. you are not using its base implementation.
• A Deeper one : you can implement many interfaces in a class so you can put the method execute in its own interface and implement it once you need it.