MVC design pattern approaches

Question

I am actually more confused rather than understanding the concept. I have been developing a system and want to know the MVC design pattern. which one is best and the concept and usability of the layers?

In general, I have two approaches:

1.

ViewModel layer (contains viewmodels), Models (contains models and datacontext), Repository (communicating to datacontext), Controller (communicating to view), Presentation (views)

In general, if one page for example, country (crude operations) will be look like this

[AdminRoleAuthorize(Roles = "Admin")][CustomHandleError] 

public class CountriesController : BaseController
{
    #region Members

    private IUnitOfWork unitOfWork;

    #endregion Members

    #region Constructor

    public CountriesController(iMSTUnitOfWork _unitOfWork)
    {
        unitOfWork = _unitOfWork;
    }

    #endregion Constructor

    #region Actions

    public ActionResult Index()
    {

        return View();
    }


    public async Task<ActionResult> Details(int? id)
    {
        if (id == null)
        {
            return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
        }
        Country country = await unitOfWork.CountryRepository.GetByIdAsync((int)id);
        if (country == null)
        {
            return HttpNotFound();
        }
        return View(country);
    }


    public ActionResult Create()
    {
        return View();
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public async Task<ActionResult> Create(Country country)
    {
        try
        {            
            if (ModelState.IsValid)
            {
                unitOfWork.CountryRepository.Insert(country);

                await unitOfWork.SaveAsync();
                return RedirectToAction("Index");
            }

            return View(country);
        }
        catch (Exception ex)
        {
            ModelState.HandleError(ex);
            return View(country);
        }
    }

    public async Task<ActionResult> Edit(int? id)
    {
        if (id == null)
        {
            return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
        }
        Country country = await unitOfWork.CountryRepository.GetByIdAsync((int)id);
        if (country == null)
        {
            return HttpNotFound();
        }
        return View(country);
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public async Task<ActionResult> Edit(Country country)
    {
        try
        {            
            if (ModelState.IsValid)
            {
                var original= unitOfWork.CountryRepository.GetById(country.Id);

                unitOfWork.CountryRepository.Update(original);
                await unitOfWork.SaveAsync();
                return RedirectToAction("Index");
            }
            return View(country);
        }
        catch (Exception ex)
        {
            ModelState.HandleError(ex);
            return View(country);
        }
    }


    public async Task<ActionResult> Delete(int id)
    {

        await unitOfWork.CountryRepository.DeleteAsync(id);
        await unitOfWork.SaveAsync();
        return RedirectToAction("Index");
    }

    #endregion Actions

    #region DataTable

    public JsonResult GetJsonData(DTParameterModel param)
    {
        try
        {
            var results = GetCountry(param);


            return Json(results, JsonRequestBehavior.AllowGet);
        }
        catch (Exception ex)
        {
            return Json(new { error = ex.Message });
        }
    }

    #endregion DataTable

    #region Methods

    public DTResult<Country> GetCountry(DTParameterModel param)
    {

        var errors = unitOfWork.CountryRepository.GetQueryable().Where(a => (a.Name.Contains(param.Search.Value) || param.Search.Value == null) );

        return DataTableFiltering<Country>.GetResult(param, errors);            

    }

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
           // unitOfWork.Dispose();
        }
        base.Dispose(disposing);
    }

    #endregion Methods



}

Please note that there is no business layer comes here and I am accessing repository directly.

2. ViewModel layer (contains viewmodels), Models (contains models and datacontext), Repository (communicating to datacontext), Controller (communicating to view), Presentation (views) Business layer

In this scenario my code will look like this:

public class ParticipationStatusController : BaseController
    {
        private readonly IParticipationStatusManager _participationStatusManager;

        public ParticipationStatusController(IParticipationStatusManager participationStatusManager, ISecurityManager securityManager) : base(securityManager)
    {
        _participationStatusManager = participationStatusManager;
    }
    #region NotUsed


    public async Task<ActionResult> ParticipationStatusListList()
    {


        var serviceResponse = new DataResponse<string> { ErrorCode = ErrorCodes.Failure };

        var clientId = GetAuthenticatedClientId();
        if (clientId == 0)
        {
            serviceResponse.Message = HttpStatusCode.Unauthorized + " :Set Authenticated Client Id";
            return View(serviceResponse);
        }

        int languageId = await GetClientLanguageId(clientId);
        var response = await _participationStatusManager.GetParticipationStatus(languageId);
        if (!response.Exceptions.Any())
        {
            if (response.Response != null)
            {
                var resultResponse = new DataResponse<IEnumerable<ParticipationStatusTranslateViewModel>>
                {
                    Data = response.Response
                };




                ErrorMessage.Getmessage(response.Response.Count(), "Participation Status", ref resultResponse);
                return Ok(resultResponse);
            }
            else
                return NotFound();
        }
        foreach (var keyValuePair in response.Exceptions)
        {
            ModelState.AddModelError(keyValuePair.Key, new Exception(keyValuePair.Value));
        }
        if (response.Response != null)
            return View(response.Response);
        else
            return BadRequest(ModelState);
    }
    #endregion





}

Please note that all the communications to database are going through the business layer which is communicating to the repository.

---

Which is a good to use and what is the role of each layer? I do not have good experience developing system design.

Also, should I use viewmodel only in presentation? and use automapper to communicate to the repository?

Answer 1

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more specific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post more code but its design doesn't look right.

It's difficult to come up with an example. The only thing I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.