Controller with too many dependencies?

The main impetus for this review is to determine if my ASP.NET Web API controller has too many dependencies. I'm of the opinion that 3-4 is okay, but 6 seems like too many. I have many controllers with similar layout. I've considered extracting all of the "PostPrimary" operations into a single wrapping dependency, but not sure if that's overkill. If I did create a wrapper, I'm not sure whether I'd create some IManipulator interface and the wrapper could just hold its own IEnumerable that it could iterate through and execute each, or whether It would just explicitly hold one of each dependency.

Some other things to note:

• This particular controller only has one "PrePrimary" manipulator, though similar controllers have more, so a PrePrimary wrapper would also be a possibility.

• I'd prefer to use some ORM instead of the repository pattern, but this legacy database is so messed up, NHibernate struggled to navigate my last project through the swamp of SQL, but the going was slow and they got bogged down many times.

• I understand that my repository's method contains waaay too many parameters, but I feel like a repository should be fairly open and honest about what constraints its going to use in its SQL, so I use a 1:1 relationship between repository parameters and SQL parameters that go into the WHERE clause. Is this reasonable?

• The verbosity of my identifiers is not open for discussion :P

So with that,

```
using System.Web.Http;
using MyProject.Models;
using MyProject.Models.DtoV1;
using MyProject.Models.Repo;

namespace MyProject.Controllers.Version1
{
public class PropertyDimensionController : ApiController
{
private readonly IPropcodeVetter propcodeVetter;
private readonly IPropertyDimensionRepository propertyDimensionRepository;
private readonly IPropertyDimensionAllPropsManager propertyDimensionAllPropsManager;
private readonly IComparisonRundateVsPopulator comparisonRundateVsPopulator;
private reado

I'm going to make some high-level suggestions about the design. Generally, what I find helpful is to clarify what each class's responsibility is. From what I understand of your code, you have 3 types of objects:

-
PropertyDimensionRequest: a data container which I assume contains the data of the web request.

-
PropertyDimensionController: this does 2 things - loads data from database, and then does an "assembly-line" kind of processing on the request and response to populate the response.

-
IPropertyDimensionAllPropsManager,IComparisonRundateVsPopulator, etc: which all populate parts of the web response.

Assuming I understood the function of these objects, I would make the following changes:

-
Modify PropertyDimensionRequest to retrieve the data and return the response. I think this better encapsulates the database parameters needed. (Why does PropertyDimensionController care what parameters are needed?) This allows gives you flexibilty to change the Data Access method, without affecting the Controller code at all.

-
Refactor the IPropertyDimensionAllPropsManager,IComparisonRundateVsPopulator, etc. to have a common interface, (maybe something called IResponsePopulator?), and add them to a list in the PropertyDimensionController class, or if they are reusable, and thread-safe, register them into some Singleton data structure.

The final code would look something like this:

public PropertyDimensionResponse Post(PropertyDimensionRequest propertyDimensionRequest)
    {
        // not sure what I would do about this
        var vettedPropcodes = propcodeVetter.GetVettedPropcodes(propertyDimensionRequest);

        // Let PropertyDimensionRequest read from database, since it contains
        // all the parameters. Just pass it objects it doesn't have access to
        var propertyDimensionResponse = propertyDimensionRequest.Read(propertyDimensionRepository, vettedPropcodes);

        // this replaces all the code where the response was being
        // processed by various objects. If refactored to a common interface
        // you just load (or inject) them into a list, and then iteratively
        // process the response
        ResponsePopulators.ForEach( i => i.Populate(propertyDimensionResponse));

        return propertyDimensionResponse;
    }
}

(My C# coding is a little rusty, so please forgive any syntax errors).

StackExchange Code Review Q#40960, answer score: 3