In my project, there are two (so far, there may be more in the future) very similar controllers.

DevelopersController

using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace H4P.Controllers
{
    public class DevelopersController : Controller
    {
        private DataManager dataManager;
        public int PageSize = 10;

        public DevelopersController(DataManager repo)
        {
            dataManager = repo;
        }

        public async Task<IActionResult> Index(int pageNumber = 1)
        {
            var developers = dataManager.DevelopersRepository.Developers.AsNoTracking();
            return View(await PaginatedList<Developer>.CreateAsync(developers, pageNumber, PageSize));
        }

        public IActionResult Developer(string id)
        {
            return View(dataManager.DevelopersRepository.GetDeveloperById(Guid.Parse(id)));
        }
    }
}

and TagsController

using H4P.Domain;
using H4P.Models;
using H4P.Models.ViewModels;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace H4P.Controllers
{
    public class TagsController : Controller
    {
        private DataManager dataManager;
        public int PageSize = 10;

        public TagsController(DataManager repo)
        {
            dataManager = repo;
        }

        public async Task<IActionResult> Index(int pageNumber = 1)
        {
            var tags = dataManager.TagsRepository.Tags.AsNoTracking();
            return View(await PaginatedList<Tag>.CreateAsync(tags, pageNumber, PageSize));
        }

        public IActionResult Tag(string id)
        {
            return View(dataManager.TagsRepository.GetTagById(Guid.Parse(id)));
        }
    }
}

At the same time, the tag and developer classes have the same parent FilterBase.

public abstract class FilterBase
{
    [Required]
    public Guid Id { get; set; }
    public virtual string Title { get; set; }
    public virtual string Text { get; set; }
    public virtual List<Game> Games { get; set; } = new List<Game>();
}

public class Tag : FilterBase
{
    //
}

public class Developer : FilterBase
{
    //
}

Therefore, at first I just wanted to combine these controllers into one FiltersController, in which the code works with the FilterBase model. But I need tags and developers to have different routes.

• .../tags - for the Index method (output all tags on the page)
• .../tag/id - for the Tag method (displaying information about a specific tag)

and similar ones for developers

• .../developers
• ../developer/id

Another option I had was to use a shared view. Those TagsController and DeveloperController would pass to this view FilterBase object. However, this will only solve the issue with views, but it will still be a problem with similar controllers.

And finally, here is my question: does this need to be refactored? And if so, how?

I will be glad to any advice. If you need more information, write and I will provide it.

CodePudding user response：

One option may be create a base controller for your Developer and Tag controller:

public class FilterController
{
    public FilterBase Filter { get; set; }
}

And create an instance of your custom filter in each controller:

public class DevelopersController : FilterController
{
    public DevelopersController(DataManager repo)
    {
        dataManager = repo;
            
        this.Filter = new Developer();
    }
}

public class TagsController : FilterController
{
    public TagsController(DataManager repo)
    {
        dataManager = repo;

        this.Filter = new Tag();
    }
}

In this way you have your routes, views... without changes and your filter. If you want (if the derived Filter class has many custom methods), you can add a derived property of the Filter class to work easily with the derived class. For example, in your DeveloperController:

public Developer DeveloperFilter => (Developer)this.Filter;

Also, if you work with dependency injection, you can inject the FilterBase property instead of create in the constructor.

CodePudding user response：

Generalization is not always good. as you can see you have two different contexts, developers and tags, looking at them the same way only because they are using CRUD actions is not a good idea. and will cause you problems in further development of system. sometimes for the sake of good separation of concepts duplicate codes are preferred.

but it can be solved in another level, for example using repository and CRUD repository implementation with accepts a TEntity as generic argument.

as you are using efcore take a look at DbContext.Set<T>() function.