Moving Controller's logic to Service layer

Viewed 47

As an exercise I was asked to moved logic from controller to service layer. With methods such as Get or GetById it wasn't a problem. Difficult part is to move Update and Patch methods. Below, you will find what I do have in Controler class and how I've changed them to the new requirement.

ToursController.cs before change

public class ToursController : ControllerBase 
{
    private readonly ToursRepository _repo;
    private readonly IMapper _mapper;

    public ToursController(ToursRepository repository, IMapper mapper)
    {
        _repo = repository ?? throw new ArgumentNullException(nameof(repository));
        _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
    }

    [HttpPut("{id}")]
    public async Task<ActionResult> Update(int id, TourForUpdateDto updateDto)
    {
        var tour = await _repo.GetByIdAsync(id);
        if (tour is null)
        {
            return NotFound();
        }

        _mapper.Map(updateDto, tour);
        await _repo.SaveChangesAsync();

        return NoContent();
    }

    [HttpPatch("{id}")]
    public async Task<ActionResult> Patch(int id, JsonPatchDocument patchDocument)
    {
        var tour = await _repo.GetByIdAsync(id);
        if (tour is null)
        {
            return NotFound();
        }

        var updateDto = _mapper.Map<TourForUpdateDto>(tour);
        patchDocument.ApplyTo(updateDto);

        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }

        if (!TryValidateModel(patchDocument))
        {
            return BadRequest(ModelState);
        }

        _mapper.Map(updateDto, tour);
        await _repo.SaveChangesAsync();

        return NoContent();
    }
}

ToursController.cs after change

public class ToursController : ControllerBase 
{
    private readonly ToursRepository _repo;
    private readonly IMapper _mapper;
    private readonly ToursService _toursService;

    public ToursController(ToursRepository repository, IMapper mapper)
    {
        _repo = repository ?? throw new ArgumentNullException(nameof(repository));
        _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
        _toursService = toursService ?? throw new ArgumentNullException(nameof(toursService));
    }

    [HttpPut("{id}")]
    public async Task<ActionResult> Update(int id, TourForUpdateDto updateDto)
    {
        if (!_toursService.Exists(id))
        {
            return NotFound();
        }

        await _toursService.Update(id, updateDto);
        return NoContent();
    }

    [HttpPatch("{id}")]
    public async Task<ActionResult> Patch(int id, JsonPatchDocument patchDocument)
    {
        // No idea how to update this method.
        var tour = await _repo.GetByIdAsync(id);
        if (tour is null)
        {
            return NotFound();
        }

        var updateDto = _mapper.Map<TourForUpdateDto>(tour);
        patchDocument.ApplyTo(updateDto);

        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }

        if (!TryValidateModel(patchDocument))
        {
            return BadRequest(ModelState);
        }

        _mapper.Map(updateDto, tour);
        await _repo.SaveChangesAsync();

        return NoContent();
    }
}

ToursService.cs after change

using AutoMapper;
using BandManager.Api.Models;
using BandManager.Api.Services.Repositories;

public class ToursService
{
    private readonly ToursRepository _repo;
    private readonly IMapper _mapper;

    public ToursService(ToursRepository repo, IMapper mapper)
    {
        _repo = repo ?? throw new ArgumentNullException(nameof(repo));
        _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
    }

    public async Task Update(int id, TourForUpdateDto updateDto)
    {
        var tour = await GetByIdAsync(id);
        if (tour is null)
            return;

        _mapper.Map(updateDto, tour);
        await _repo.SaveChangesAsync();
    }

    public async Task<TourDto?> GetByIdAsync(int id)
    {
        return _mapper.Map<TourDto>(await _repo.GetByIdAsync(id));
    }

    public bool Exists(int id)
    {
        return _repo.Exists(id);
    }
}

Questions and thoughts

  • Should I ever return objects which are implementing IActionResult or any MVC related types from my services? My gut feeling is - no.
  • Should service be able to use AutoMapper to convert entities to DTOs? I think, yes.
  • Should my service, in the same method call, be able to call SaveChangesAsync on repository?
0 Answers
Related