3
\$\begingroup\$

I'm trying out .net core with mvc in a personal project, the latest functionality I've added was a project catalog, primarily ajax based, it's displaying a list of projects fetched from the database using web api in a catalogish order and interface.


Web API

Starting with the infrastructure layer, I decided to have 2 classes, obviously one api controller and one http wrapper service, to allow easier usage of the web api. For this part of the project there are 2 necessary services - ProjectService and ProjectFavoritesService, the first one contains crud operations for the project objects and project favorites is a junction table for User and Project and allows for projects to be favorited by a user.

IProjectsService

public interface IProjectsService
{
    Task<int> AddProject(ProjectsDTO dto);
    Task<int> ModifyProject(ProjectsDTO dto);
    Task RemoveProject(int id);
    Task<ProjectsDTO> GetProject(int id);
    Task<List<ProjectsDTO>> GetProjects();
}

ProjectsController

[Route("api/[controller]")]
[ApiController]
public class ProjectsController : ControllerBase
{
    private readonly DexContext _context;
    private readonly IMapper _mapper;

    public ProjectsController(DexContext context, IMapper mapper)
    {
        _context = context;
        _mapper = mapper;
    }

    [HttpPost]
    [Route("AddProject")]
    public IActionResult AddProject(ProjectsDTO dto)
    {
        if (ModelState.IsValid)
        {
            try
            {
                var entity = _mapper.Map<Projects>(dto);
                _context.Projects.Add(entity);
                _context.SaveChanges();

                if (entity.Id > 0)
                {
                    return Ok(entity.Id);
                }
            }
            catch (Exception)
            {
                return BadRequest();
            }
        }

        return BadRequest();
    }

    [HttpPut]
    [Route("ModifyProject")]
    public IActionResult ModifyProject(ProjectsDTO dto)
    {
        if (ModelState.IsValid)
        {
            try
            {
                var entity = _context.Projects.FirstOrDefault(p => p.Id == dto.Id);
                if (entity == null)
                {
                    return NotFound();
                }

                _mapper.Map(dto, entity);
                _context.SaveChanges();

                return Ok(entity.Id);
            }
            catch (Exception)
            {
                return BadRequest();
            }
        }

        return BadRequest();
    }

    [HttpDelete]
    [Route("RemoveProject")]
    public IActionResult RemoveProject(int? id)
    {
        if(id == null) { return BadRequest(); }

        var entity = _context.Projects.FirstOrDefault(p => p.Id == id);
        if (entity == null)
        {
            return NotFound();
        }

        try
        {
            _context.Projects.Remove(entity);
            _context.SaveChanges();
            return Ok();
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }

    [HttpGet]
    [Route("GetProject")]
    public IActionResult GetProject(int? id)
    {
        if (id == null) { return BadRequest(); }

        try
        {
            var project = _context.Projects.FirstOrDefault(p => p.Id == id);
            if (project == null)
            {
                return NotFound();
            }

            return Ok(_mapper.Map<ProjectsDTO>(project));
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }

    [HttpGet]
    [Route("GetProjects")]
    public IActionResult GetProjects()
    {
        if (_context == null)
        {
            return NotFound();
        }
        try
        {
            return Ok(_context.Projects.Select(p => _mapper.Map<ProjectsDTO>(p)));
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }
}

ProjectFavoritesController

[Route("api/[controller]")]
[ApiController]
public class ProjectFavoritesController : ControllerBase
{
    private readonly DexContext _context;
    private readonly IMapper _mapper;

    public ProjectFavoritesController(DexContext context, IMapper mapper)
    {
        _context = context;
        _mapper = mapper;
    }

    [HttpPost]
    [Route("AddFavorite")]
    public IActionResult AddFavorite(ProjectFavoritesDTO dto)
    {
        if (ModelState.IsValid)
        {
            try
            {
                var entity = _mapper.Map<ProjectFavorites>(dto);
                _context.ProjectFavorites.Add(entity);
                _context.SaveChanges();

                return Ok();
            }
            catch (Exception)
            {
                return BadRequest();
            }
        }

        return BadRequest();
    }

    [HttpDelete]
    [Route("RemoveFavorite")]
    public IActionResult RemoveFavorite(ProjectFavoritesDTO dto)
    {
        if (dto == null) { return BadRequest(); }

        try
        {
            var entity = _context.ProjectFavorites.SingleOrDefault(p => p.ProjectId == dto.ProjectId && p.UserId == dto.UserId);
            if (entity == null)
            {
                return NotFound();
            }

            _context.ProjectFavorites.Remove(entity);
            _context.SaveChanges();
            return Ok();
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }

    [HttpGet]
    [Route("GetFavoritesByUser")]
    public IActionResult GetFavoritesByUser(string userId)
    {
        if (_context == null || userId == null)
        {
            return NotFound();
        }

        try
        {
            return Ok(_context.ProjectFavorites.Where(p => p.UserId == userId)
                .Select(p => _mapper.Map<ProjectFavoritesDTO>(p)));
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }

    [HttpGet]
    [Route("GetFavoritesByProject")]
    public IActionResult GetFavoritesByProject(int? projectId)
    {
        if (_context == null || projectId == null)
        {
            return NotFound();
        }

        try
        {
            return Ok(_context.ProjectFavorites.Where(p => p.ProjectId == projectId)
                .Select(p => _mapper.Map<ProjectFavoritesDTO>(p)));
        }
        catch (Exception)
        {
            return BadRequest();
        }
    }
}

Web API Client Services

ProjectsService

public class ProjectsService : IProjectsService
{
    private readonly string _host;
    private const string ControllerAddress = "api/projects/";

    public ProjectsService(IConfiguration configuration)
    {
        if (configuration == null) { throw new ArgumentNullException(nameof(configuration)); }

        _host = configuration.GetSection("Data").GetSection("Host").Value;
    }

    public async Task<int> AddProject(ProjectsDTO dto)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        var response = await client.PostAsJsonAsync(nameof(ProjectsController.AddProject), dto);

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsStringAsync();
            if (int.TryParse(message, out var id))
            {
                return id;
            }
        }

        return -1;
    }

    public async Task<int> ModifyProject(ProjectsDTO dto)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        var response = await client.PutAsJsonAsync(nameof(ProjectsController.ModifyProject), dto);

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsStringAsync();
            if (int.TryParse(message, out var id))
            {
                return id;
            }
        }

        return -1;
    }

    public async Task RemoveProject(int id)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        await client.DeleteAsync($"{nameof(ProjectsController.RemoveProject)}?id={id}");
    }

    public async Task<ProjectsDTO> GetProject(int id)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        var response = await client.GetAsync($"{nameof(ProjectsController.GetProject)}?id={id}");

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsJsonAsync<ProjectsDTO>();
            return message;
        }

        return null;
    }

    public async Task<List<ProjectsDTO>> GetProjects()
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        var response = await client.GetAsync(nameof(ProjectsController.GetProjects));

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectsDTO>>();
            return message;
        }

        return null;
    }
}

ProjectFavoritesService

public class ProjectFavoritesService : IProjectFavoritesService
{
    private readonly string _host;
    private const string ControllerAddress = "api/projectfavorites/";

    public ProjectFavoritesService(IConfiguration configuration)
    {
        if (configuration == null) { throw new ArgumentNullException(nameof(configuration)); }

        _host = configuration.GetSection("Data").GetSection("Host").Value;
    }

    public async Task AddFavorite(ProjectFavoritesDTO dto)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        await client.PostAsJsonAsync(nameof(ProjectFavoritesController.AddFavorite), dto);
    }

    public async Task RemoveFavorite(ProjectFavoritesDTO dto)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        await client.DeleteAsJsonAsync(nameof(ProjectFavoritesController.RemoveFavorite), dto);
    }

    public async Task<List<ProjectFavoritesDTO>> GetFavoritesByUser(string userId)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };
        var response = await client.GetAsync($"{nameof(ProjectFavoritesController.GetFavoritesByUser)}?userId={userId}");

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectFavoritesDTO>>();
            return message;
        }

        return null;
    }

    public async Task<List<ProjectFavoritesDTO>> GetFavoritesByProject(int projectId)
    {
        using var client = new HttpClient { BaseAddress = new Uri(_host + ControllerAddress) };

        var response = await client.GetAsync($"{nameof(ProjectFavoritesController.GetFavoritesByProject)}?projectId={projectId}");

        if (response.IsSuccessStatusCode)
        {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectFavoritesDTO>>();
            return message;
        }

        return null;
    }
}

MVC

I'm using the standard mvc approach here instead of using razor pages, due to some limitations with the latter.

DownloadsController

public class DownloadsController : Controller
{
    private const int ItemsPerPage = 10;

    private readonly IMapper _mapper;
    private readonly IProjectsService _projectsService;
    private readonly IProjectFavoritesService _projectFavoritesService;
    private readonly UserManager<AspNetUsers> _userManager;

    private readonly PagingHandler _pagingHandler;

    public DownloadsController(
        IProjectsService projectsService,
        IProjectFavoritesService projectFavoritesService,
        UserManager<AspNetUsers> userManager,
        IMapper mapper)
    {
        _mapper = mapper;
        _projectsService = projectsService;
        _projectFavoritesService = projectFavoritesService;
        _userManager = userManager;
        _pagingHandler = new SearchHandler();
        _pagingHandler.SetNext(new SortHandler());
    }

    [HttpGet]
    public IActionResult Index(int pageNumber = 1)
    {
        TempData["currentPage"] = pageNumber;
        return View();
    }

    [HttpGet]
    public async Task<JsonResult> GetProjects(PagingData data)
    {
        TempData["searchCriteria"] = data.SearchCriteria; //keep search after refresh in ui
        var currentUser = await GetCurrentUserAsync();
        var userFavorites = await _projectFavoritesService.GetFavoritesByUser(currentUser?.Id);
        var allProjects = (await _projectsService.GetProjects()).Select(p =>
        {
            var vm = _mapper.Map<ProjectsDTO, ProjectViewModel>(p);
            vm.IsFavorite = currentUser != null && userFavorites.Any(f => f.ProjectId == p.Id);

            return vm;
        });

        var projects = _pagingHandler.Handle(allProjects, data);

        var currentProjects = projects
            .Skip(ItemsPerPage * (data.CurrentPage - 1))
            .Take(ItemsPerPage);

        return Json(new
        {
            items = currentProjects,
            maxPages = GetMaxPages(projects),
        });
    }

    [HttpGet]
    public IActionResult GetPartialProject(string json)
    {
        return PartialView("_ProjectPartial", JsonConvert.DeserializeObject<ProjectViewModel>(json));
    }

    [HttpGet]
    public IActionResult GetPartialNoResults()
    {
        return PartialView("_NoResultsPartial");
    }

    [Authorize(Policy = "User")]
    [HttpGet]
    public async Task<IActionResult> SetFavoriteProject(int projectId)
    {
        var currentUser = await GetCurrentUserAsync();
        if (currentUser == null)
        {
            return Json(new {success = false});
        }

        await _projectFavoritesService.AddFavorite(new ProjectFavoritesDTO
            {UserId = currentUser.Id, ProjectId = projectId});

        return Json(new {success = true});
    }

    [Authorize(Policy = "User")]
    [HttpGet]
    public async Task<IActionResult> RemoveFavoriteProject(int projectId)
    {
        var currentUser = await GetCurrentUserAsync();
        if (currentUser == null)
        {
            return Json(new {success = false});
        }

        await _projectFavoritesService.RemoveFavorite(new ProjectFavoritesDTO
            {UserId = currentUser.Id, ProjectId = projectId});

        return Json(new {success = true});
    }

    private int GetMaxPages(IEnumerable<ProjectViewModel> source)
    {
        return source == null ? 1 : (int) Math.Ceiling(source.Count() / (double) ItemsPerPage);
    }

    private Task<AspNetUsers> GetCurrentUserAsync() => _userManager.GetUserAsync(HttpContext.User);
}

Index.cshtml

@inject SignInManager<AspNetUsers> SignInManager
@using Dex.Common.Resources
@using Dex.DataAccess.Models
@using Microsoft.AspNetCore.Identity
@{
    Layout = "_Layout";
    var searchCriteria = (string)TempData["searchCriteria"];
}

<br />

<div>
    <div class="float-left" style="margin-left: 45px;">
        <a id = "sortAsc" class="selected-arrow">
            <i class="fa fa-arrow-up"></i>
        </a>
        <a id = "sortDesc" class="unselected-arrow">
            <i class="fa fa-arrow-down"></i>
        </a>
        <div class="mainmenu">
            <ul id = "sortCategories" >
                < li >
                    < a href="#">
                        Sort
                        <i class="fa fa-long-arrow-down "></i>
                    </a>
                    <ul>
                        <li>
                            <a href = "#" class="sort">Name</a>
                        </li>
                        <li>
                            <a href = "#" class="sort">Date</a>
                        </li>
                        @if(SignInManager.IsSignedIn(User))
{
                            < li >
                                < a href = "#" class="sort">Favorite</a>
                            </li>
                        }
                    </ul>
                </li>
            </ul>
        </div>
    </div>
    <div class="float-right" style="margin-right: 45px;">
        <input id = "searchBar" class="form-control mr-sm-2" type="text" placeholder="Search project" aria-label="Search" value="@searchCriteria">
    </div>

</div>

<br />
<br />

<div id = "projectsListDiv" class="box">
</div>

<div id = "paginationListDiv" class="text-center">
</div>

@section Scripts
{
    <script>
        $(document).ready(function () {
        if (window.sessionStorage.getItem('currentPage') === null)
        {
            initializePagingVariables();
        }
        var maxPages = 1;

        refreshProjectsList();
        updateSortArrows();

            $('#searchBar').on('input',
                delay(function() {
            window.sessionStorage.setItem('searchCriteria', $('#searchBar').val());
            window.sessionStorage.setItem('currentPage', 1); //reset pages

            refreshProjectsList();
        },
                    100));

            $('#sortAsc').on('click',
                function() {
            window.sessionStorage.setItem('currentPage', 1); //reset pages
            window.sessionStorage.setItem('isAscending', true);

            refreshProjectsList();
            updateSortArrows();
        });

            $('#sortDesc').on('click',
                function() {
            window.sessionStorage.setItem('currentPage', 1); //reset pages
            window.sessionStorage.setItem('isAscending', false);

            refreshProjectsList();
            updateSortArrows();
        });

            $('#sortCategories').on('click', 'a.sort',
                function() {
            window.sessionStorage.setItem('sort', $(this).text());
            refreshProjectsList();
        });

            $('#paginationListDiv').on('click', 'a.page-navigator',
                function() {
            var element = $(this);
            var currentPage = parseInt(window.sessionStorage.getItem('currentPage'));
            if (currentPage > 1 && element.attr('id') == 'previousPage')
            {
                window.sessionStorage.setItem('currentPage', currentPage - 1);
            }
            else if (currentPage < maxPages && element.attr('id') == 'nextPage')
            {
                window.sessionStorage.setItem('currentPage', currentPage + 1);
            }
            else if (element.attr('id').indexOf('page') >= 0)
            {
                window.sessionStorage.setItem('currentPage', element.attr('id').substring('page'.length));
            }

            refreshProjectsList();
        });

        function refreshProjectsList()
        {
            var request = getProjectsAjax();
            loadProjectsFromAjax(request);
        }

        function getProjectsAjax()
        {
            return $.ajax({
            url: '/Downloads/GetProjects',
                    method: 'GET',
                    accepts: 'text/json',
                    data:
                {
                currentPage: window.sessionStorage.getItem('currentPage'),
                        searchCriteria: window.sessionStorage.getItem('searchCriteria'),
                        sort: window.sessionStorage.getItem('sort'),
                        isAscending: window.sessionStorage.getItem('isAscending')
                    },
                    headers:
                {
                RequestVerificationToken:
                            $('input:hidden[name="__RequestVerificationToken"]').val()
                    }
            });
    }

    function ajaxFavorite(resourceName, projectId) {
        return $.ajax({
        url: '/Downloads/' + resourceName,
                    method: 'GET',
                    accepts: 'text/json',
                    data:
            {
            projectId: projectId
                    },
                    headers:
            {
            RequestVerificationToken:
                            $('input:hidden[name="__RequestVerificationToken"]').val()
                    }
        });
    }

    function loadProjectsFromAjax(ajaxRequest) {
        ajaxRequest.done(function(data) {
            if (data.items.length > 0)
            {
                appendProjectItems(data.items);
                appendPaginationItems(window.sessionStorage.getItem('currentPage'), data.maxPages);
            }
            else
            {
                loadNoResultView();
            }
        });
    }

    document.addEventListener('click',
                function (e) {
        if (hasClass(e.target, 'favoriteStar'))
        {
            var resourceName = 'RemoveFavoriteProject';
            if (hasClass(e.target, 'unchecked'))
            {
                resourceName = 'SetFavoriteProject';
            }

            var classes = e.target.className.split(' ');
            var projectId = 0;
            var classPrefix = 'project-';
            for (var i = 0; i < classes.length; i++)
            {
                if (classes[i].indexOf(classPrefix) >= 0)
                {
                    projectId = classes[i].substring(classPrefix.length);
                    break;
                }
            }

            var request = ajaxFavorite(resourceName, projectId);
            request.fail(function(jqXHR) {
                if (jqXHR.status === 401)
                {
                                $.growl.warning({ message: 'You need to be logged in to set favorites ' });
                }
            });

            request.done(function(data) {
                if (data.success)
                {
                    flipStarClasses(e.target);
                }
            });
        }
    }, false);

    function appendProjectItems(data) {
        var list = $('<ul id="projectsList" class="text-center p-0">');
                $('#projectsListDiv').html(list);

        var promises = data.map(function(e) {
            return new Promise(function(resolve) {
                        $.get('/Downloads/GetPartialProject',
                            { json: JSON.stringify(e) },
                            function (response) {
                                var htmlElement = '<li class="horizontal-li box faded-out">' + response + '</li>';
            resolve(htmlElement);
        });
    });
});

                Promise.all(promises).then(function (promiseResults) {
                    $('#projectsList').append(promiseResults);
});

                list.append('</ul>');
                window.getComputedStyle(document.getElementById('projectsList')).opacity; //forces batch
            }

            function appendPaginationItems(currentPage, totalPages)
{
    maxPages = totalPages;
    var list = $('<ul id="paginationListDiv" class="pagination">');
    list.append(
        '<li><a href="#" id="previousPage" class="box faded-out page-navigator">«</a></li>');
                $('#paginationListDiv').html(list);
    for (var i = 1; i <= maxPages; i++)
    {
        if (currentPage == i)
        {
            list.append('<li><a href="#" id="page' +
                i +
                '" class="active box faded-out page-navigator"</a>' +
                i +
                '</li>');
        }
        else
        {
            list.append('<li><a href="#" id="page' +
                i +
                '"class="box faded-out page-navigator" </a>' +
                i +
                '</li>');
        }
    }
    list.append(
        '<li><a href="#" id="nextPage" class="box faded-out page-navigator">»</a></li>');
    list.append('</ul>');
}

function updateSortArrows()
{
    if (window.sessionStorage.getItem('isAscending') === 'true')
    {
                    $('#sortAsc').removeClass('unselected-arrow');
                    $('#sortAsc').addClass('selected-arrow');
                    $('#sortDesc').removeClass('selected-arrow');
                    $('#sortDesc').addClass('unselected-arrow');
    }
    else
    {
                    $('#sortDesc').removeClass('unselected-arrow');
                    $('#sortDesc').addClass('selected-arrow');
                    $('#sortAsc').removeClass('selected-arrow');
                    $('#sortAsc').addClass('unselected-arrow');
    }
}

function loadNoResultView()
{
                $.get('/Downloads/GetPartialNoResults',
                    function(response) {
                        $('#projectsListDiv').html(response);
                        $('#paginationListDiv').html('');
    });
}

function initializePagingVariables()
{
    window.sessionStorage.setItem('searchCriteria', $('#searchBar').val());
    window.sessionStorage.setItem('currentPage', 1);
    window.sessionStorage.setItem('sort', 'name');
    window.sessionStorage.setItem('isAscending', true);
}

function flipStarClasses(e)
{
    if (hasClass(e, 'checked'))
    {
        e.classList.remove("checked");
        e.classList.remove("bounceIn");
        e.classList.add("unchecked");
    }
    else
    {
        e.classList.remove("unchecked");
        e.classList.add("checked");
        e.classList.add("bounceIn");
    }
}
        });
    </script>
}

To allow for an easier paging in the controller, there are a couple helper classes, implemented using a slightly modified chain of responsibility:

public interface IHandler<T>
{
    IHandler<T> SetNext(IHandler<T> handler);

    T Handle(T request, object additionalData);
}

public abstract class PagingHandler : IHandler<IEnumerable<ProjectViewModel>>
{
    private IHandler<IEnumerable<ProjectViewModel>> _nextHandler;

    public IHandler<IEnumerable<ProjectViewModel>> SetNext(
        IHandler<IEnumerable<ProjectViewModel>> handler)
    {
        this._nextHandler = handler;
        return handler;
    }

    public virtual IEnumerable<ProjectViewModel> Handle(IEnumerable<ProjectViewModel> request,
        object additionalData)
    {
        if (_nextHandler == null)
        {
            return request;
        }

        return _nextHandler.Handle(request, additionalData);
    }
}

public class SearchHandler : PagingHandler
{
    public override IEnumerable<ProjectViewModel> Handle(IEnumerable<ProjectViewModel> request,
        object additionalData)
    {
        if (additionalData is PagingData pagingData && !string.IsNullOrEmpty(pagingData.SearchCriteria))
        {
            return base.Handle(
                request.Where(p =>
                    p.ProjectName.Contains(pagingData.SearchCriteria, StringComparison.OrdinalIgnoreCase)),
                additionalData);
        }

        return base.Handle(request, additionalData);
    }
}

public class SortHandler : PagingHandler
{
    public override IEnumerable<ProjectViewModel> Handle(IEnumerable<ProjectViewModel> request,
        object additionalData)
    {
        if (additionalData is PagingData pagingData && !string.IsNullOrEmpty(pagingData.Sort))
        {
            return base.Handle(
                ProjectSortResolver.Resolve(pagingData.Sort).Sort(request, pagingData.IsAscending),
                additionalData);
        }

        return base.Handle(request, additionalData);
    }
}

public static class ProjectSortResolver
{
    private static readonly Dictionary<string, ISortStrategy<ProjectViewModel>> _sortStrategies;

    static ProjectSortResolver()
    {
        _sortStrategies =
            new Dictionary<string, ISortStrategy<ProjectViewModel>>(StringComparer.OrdinalIgnoreCase)
            {
                ["name"] = new ProjectNameSortStrategy(),
                ["date"] = new ProjectDateSortStrategy(),
                ["favorite"] = new ProjectFavoriteSortStrategy(),

            };
    }

    public static ISortStrategy<ProjectViewModel> Resolve(string type)
    {
        return _sortStrategies.TryGetValue(type, out var strategy) ? strategy : null;
    }
}

public interface ISortStrategy<T>
{
    IEnumerable<T> Sort(IEnumerable<T> data, bool isAscending);
}

public class ProjectNameSortStrategy : ISortStrategy<ProjectViewModel>
{
    public IEnumerable<ProjectViewModel> Sort(IEnumerable<ProjectViewModel> data, bool isAscending)
    {
        if (isAscending)
        {
            return data.OrderBy(p => p.ProjectName);
        }

        return data.OrderByDescending(p => p.ProjectName);
    }
}

public class ProjectDateSortStrategy : ISortStrategy<ProjectViewModel>
{
    public IEnumerable<ProjectViewModel> Sort(IEnumerable<ProjectViewModel> data, bool isAscending)
    {
        if (isAscending)
        {
            return data.OrderBy(p => p.ProjectDate);
        }

        return data.OrderByDescending(p => p.ProjectDate);
    }
}

public class ProjectFavoriteSortStrategy : ISortStrategy<ProjectViewModel>
{
    public IEnumerable<ProjectViewModel> Sort(IEnumerable<ProjectViewModel> data, bool isAscending)
    {
        if (isAscending)
        {
            return data.OrderBy(p => p.IsFavorite);
        }

        return data.OrderByDescending(p => p.IsFavorite);
    }
}

public class PagingData
{
    public int CurrentPage { get; set; }
    public string SearchCriteria { get; set; }
    public string Sort { get; set; }
    public bool IsAscending { get; set; }
}

I'm primarily looking for reviews focusing on the overall design of the functionality, but anything else is also welcome!

\$\endgroup\$
  • 1
    \$\begingroup\$ AddProject, ModifyProject, RemoveProject, GetProject I think it would be better if you just use Add, Modify, Remove, Get since its under Project, there is no need to repeat it in each method. same goes for API. Also, GetProject(int id) and GetProjects(), just use Get() and Get(int Id), so when I call api/projects it'll return all projects, and if i call api/projects/10 it'll return project number 10 and so on. Also, ProjectFavoritesService is part of ProjectService those, in the API it seems to be reasonable to do api/projects/favorites \$\endgroup\$ – iSR5 Dec 12 '19 at 5:24
2
\$\begingroup\$

Injecting IConfiguration is more and more being seen as an indicator of Single Responsibility Principle (SRP) violation when the framework allows for a more SOLID approach of dealing with configuration at the composition root.

Also the creation of new HttpClient manually can cause issues

Reference You're using HttpClient wrong

Both API service implementations have done this. Both implementations only need the client injected so it should be configured to do so

ConfigureServices

var host = Configuration.GetSection("Data").GetSection("Host").Value;
projectsControllerBaseAddress = new Uri(host + "api/projects/");
projectsFavoritesControllerBaseAddress = new Uri(host + "api/projectfavorites/");

//Register Typed clients
services.AddHttpClient<IProjectsService, ProjectService>(client => {
    client.BaseAddress = projectsControllerBaseAddress;

    //...any other settings needed
});

services.AddHttpClient<IProjectFavoritesService, ProjectFavoritesService>(client => {
    client.BaseAddress = projectsFavoritesControllerBaseAddress;

    //...any other settings needed
})

Reference How to use Typed Clients with HttpClientFactory

This removes the need for creating the clients within the services as it will be injected by the framework that will also manage its lifetime.

ProjectService Refactored:

public class ProjectsService : IProjectsService {
    private readonly HttpClient client;

    public ProjectsService(HttpClient client) {
        if (client == null) { throw new ArgumentNullException(nameof(client)); }
        this.client = client;
    }

    public async Task<int> AddProject(ProjectsDTO dto) {
        var response = await client.PostAsJsonAsync(nameof(ProjectsController.AddProject), dto);
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsStringAsync();
            if (int.TryParse(message, out var id)) {
                return id;
            }
        }
        return -1;
    }

    public async Task<int> ModifyProject(ProjectsDTO dto) {
        var response = await client.PutAsJsonAsync(nameof(ProjectsController.ModifyProject), dto);
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsStringAsync();
            if (int.TryParse(message, out var id)) {
                return id;
            }
        }
        return -1;
    }

    public async Task RemoveProject(int id) {
        await client.DeleteAsync($"{nameof(ProjectsController.RemoveProject)}?id={id}");
    }

    public async Task<ProjectsDTO> GetProject(int id) {
        var response = await client.GetAsync($"{nameof(ProjectsController.GetProject)}?id={id}");
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsJsonAsync<ProjectsDTO>();
            return message;
        }
        return null; //Should consider NullObject pattern
    }

    public async Task<List<ProjectsDTO>> GetProjects() {
        var response = await client.GetAsync(nameof(ProjectsController.GetProjects));
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectsDTO>>();
            return message;
        }
        return new List<ProjectsDTO>(); //Return empty list to avoid null errors.
    }
}

ProjectFavoritesService Refactored

public class ProjectFavoritesService : IProjectFavoritesService {
    private readonly HttpClient client;

    public ProjectFavoritesService(IConfiguration configuration) {
        if (configuration == null) { throw new ArgumentNullException(nameof(configuration)); }
        this.client = client;
    }

    public async Task AddFavorite(ProjectFavoritesDTO dto) {
        await client.PostAsJsonAsync(nameof(ProjectFavoritesController.AddFavorite), dto);
    }

    public async Task RemoveFavorite(ProjectFavoritesDTO dto) {
        await client.DeleteAsJsonAsync(nameof(ProjectFavoritesController.RemoveFavorite), dto);
    }

    public async Task<List<ProjectFavoritesDTO>> GetFavoritesByUser(string userId) {
        var response = await client.GetAsync($"{nameof(ProjectFavoritesController.GetFavoritesByUser)}?userId={userId}");
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectFavoritesDTO>>();
            return message;
        }
        return List<ProjectFavoritesDTO>();
    }

    public async Task<List<ProjectFavoritesDTO>> GetFavoritesByProject(int projectId) {
        var response = await client.GetAsync($"{nameof(ProjectFavoritesController.GetFavoritesByProject)}?projectId={projectId}");
        if (response.IsSuccessStatusCode) {
            var message = await response.Content.ReadAsJsonAsync<List<ProjectFavoritesDTO>>();
            return message;
        }
        return new List<ProjectFavoritesDTO>();
    }
}

Your service functions returning lists should return an empty list to avoid null errors and you should also consider using Null Object Pattern for functions returning reference types.

|improve this answer|||||
\$\endgroup\$
  • 1
    \$\begingroup\$ Also, another good HttpClient reference: josefottosson.se/… \$\endgroup\$ – Jesse C. Slicer Dec 12 '19 at 14:41
  • \$\begingroup\$ I wasn't feeling comfortable with the way the http client was handled either, this solves it! Great points! \$\endgroup\$ – Denis Dec 12 '19 at 21:05
  • \$\begingroup\$ It's also a great solution because it's consistent with the rest of the code, I also configure and inject the dbcontext in the same way, I don't know why I didn't thought of that :) \$\endgroup\$ – Denis Dec 13 '19 at 0:00
2
+100
\$\begingroup\$

Returning early if a specific condition is met will make a method more readable because the level of indentation is decreased. E.g in ProjectsController.AddProject(ProjectsDTO) you could revert the if condition at the top of the method like so

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

and because you return BadRequest() in case of an exception you could have an empty catch block but adding a finally to return BadRequest() for the case that an exception is thrown and for entity.Id <= 0.

This would look like so

[HttpPost]
[Route("AddProject")]
public IActionResult AddProject(ProjectsDTO dto)
{
    if (!ModelState.IsValid) { return BadRequest(); }

    try
    {
        var entity = _mapper.Map<Projects>(dto);
        _context.Projects.Add(entity);
        _context.SaveChanges();

        if (entity.Id > 0)
        {
            return Ok(entity.Id);
        }
    }
    catch {} // swallowing exception, but returning BadRequest()

    return BadRequest();
}  

This pattern should be used in ProjectsController.ModifyProject() and ProjectFavoritesController.AddFavorite() as well.

I find it a little bit odd that you need to check if _context == null in ProjectsController.GetProjects() but for each other method you don't check the state of that field. IMO you should validate this inside the constructor and throwing an ArgumentNullException if the passed context is null.
This applies for ProjectFavoritesController as well.

In both AddProject() and ModifyProject you don't validate the method argument. Its a public method hence you really should do it. This applies for AddFavorite() of the ProjectFavoritesController as well.

In ProjectFavoritesController.GetFavoritesByUser() you check if the passed in userId is null, but what about an empty string ?

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Consistency and code style are important factors for me, thank you for the suggestions! \$\endgroup\$ – Denis Dec 12 '19 at 23:44
  • \$\begingroup\$ Just one thing, finally blocks cannot return value in C#. \$\endgroup\$ – Denis Dec 12 '19 at 23:46
  • \$\begingroup\$ Wow, good catch. Updated answer. \$\endgroup\$ – Heslacher Dec 13 '19 at 4:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.