Converting web-querying classes to a single generic class

Question

I have two classes that do that same thing with different types. I was thinking of using generics (but I am open to all options). QueryPermissions and the WebQueryObjects come from an external dll that I cannot edit.

 public class OnlineReportOutputService : AService, IOnlineReportOutputService
{
    public IEnumerable<OnlineReportOutput> GetBy(int id, int type, string reportName)
    {
        var webQueryOutputs = GetOutputs(id, type, reportName);
        return Mapper.Map<List<WebQueryOutput>, List<OnlineReportOutput>>(webQueryOutputs);
    }

    public StatusReturn Add(IEnumerable<OnlineReportOutput> onlineReportOutputs)
    {
        return AddOrUpdate(onlineReportOutputs);
    }

    public StatusReturn Update(IEnumerable<OnlineReportOutput> onlineReportOutputs)
    {
        return AddOrUpdate(onlineReportOutputs);
    }

    //Slight flaw in logic. Returning a List<StatusReturn> may be a better option but still doesn't fit rules
    //TODO: Get with original developer and see if there is a better return value
    public StatusReturn Delete(IEnumerable<OnlineReportOutput> onlineReportOutputs)
    {
        var statusReturn = new StatusReturn();
        var webQueryOutputs = Mapper.Map<IEnumerable<OnlineReportOutput>, List<WebQueryOutput>>(onlineReportOutputs);
        foreach (var webQueryOutput in webQueryOutputs)
        {
            statusReturn = _queryPermissions.DeleteWebQueryOutput(webQueryOutput);
            if (statusReturn.StatusCode.ToInt() == 500) return statusReturn;
        }
        return statusReturn;
    }

    private StatusReturn AddOrUpdate(IEnumerable<OnlineReportOutput> onlineReportOutputs)
    {
        var webQueryOutputs = Mapper.Map<IEnumerable<OnlineReportOutput>, List<WebQueryOutput>>(onlineReportOutputs);
        return _queryPermissions.UpdateWebQueryOutputs(webQueryOutputs, false);
    }

    private List<WebQueryOutput> GetOutputs(int id, int type, string reportName)
    {
        var webQueryOutputs = new List<WebQueryOutput>();
        _queryPermissions.GetWebQueryOutputs((QueryPermission.WebQueryTypes)type, ref webQueryOutputs, id, reportName);
        return webQueryOutputs;
    }
}


public class OnlineReportInputService : AService, IOnlineReportInputService
{
    public StatusReturn Add(IEnumerable<OnlineReportInput> onlineReportInputProperties)
    {
        return AddAndUpdate(onlineReportInputProperties);
    }

    public IEnumerable<OnlineReportInput> GetBy(int id, int type, string reportName)
    {
        var webQueryInputs = GetInputs(id, type, reportName);
        return Mapper.Map<IEnumerable<WebQueryInput>, List<OnlineReportInput>>(webQueryInputs);
    }

    public StatusReturn Update(IEnumerable<OnlineReportInput> onlineReportInputs)
    {
        return AddAndUpdate(onlineReportInputs);
    }

    //Slight flaw in logic. Returning a List<StatusReturn> may be a better option but still doesn't fit rules
    //TODO: Get with original developer and see if there is a better return value
    public StatusReturn Delete(IEnumerable<OnlineReportInput> onlineReportInputs)
    {
        var statusReturn = new StatusReturn();
        var webQueryInputs = Mapper.Map<IEnumerable<OnlineReportInput>, List<WebQueryInput>>(onlineReportInputs);
        foreach (var webQueryInput in webQueryInputs)
        {
            statusReturn = _queryPermissions.DeleteWebQueryInput(webQueryInput);
            if (statusReturn.StatusCode.ToInt() == 500) return statusReturn;
        }
        return statusReturn;
    }

    private StatusReturn AddAndUpdate(IEnumerable<OnlineReportInput> onlineReportInputProperties)
    {
        var webQueryInputs = Mapper.Map<IEnumerable<OnlineReportInput>, List<WebQueryInput>>(onlineReportInputProperties);
        return _queryPermissions.UpdateWebQueryInputs(webQueryInputs, false);
    }

    private IEnumerable<WebQueryInput> GetInputs(int id, int type, string reportName)
    {
        var webQueryInputs = new List<WebQueryInput>();
        _queryPermissions.GetWebQueryInputs((QueryPermission.WebQueryTypes)type, ref webQueryInputs, id, reportName);
        return webQueryInputs;
    }
}

What I have so far:

 public class OnlineReportChildService<T> : IOnlineReportChildService<T>
{
    private QueryPermission _queryPermissions;

    public OnlineReportChildService()
    {
        _queryPermissions = new QueryPermission();
    }

    public IEnumerable<T> GetBy(int id, int type, string reportName)
    {
        var webQueryOutputs = GetOutputs(id, type, reportName);
        //Not sure how to make Automapper calls generically (AMC)
        return Mapper.Map<List<WebQueryOutput>, List<OnlineReportOutput>>(webQueryOutputs);
    }

    public StatusReturn Add(IEnumerable<T> onlineReportChildren)
    {
        return AddOrUpdate(onlineReportChildren);
    }

    public StatusReturn Update(IEnumerable<T> onlineReportChildren)
    {
        return AddOrUpdate(onlineReportChildren);
    }

    //Slight flaw in logic. Returning a List<StatusReturn> may be a better option but still doesn't fit rules
    //TODO: Get with original developer and see if there is a better return value
    public StatusReturn Delete(IEnumerable<T> onlineReportChildren)
    {
        var statusReturn = new StatusReturn();
        //AMC
        var webQueryChildren = Mapper.Map<IEnumerable<OnlineReportOutput>, List<WebQueryOutput>>(onlineReportChildren);
        foreach (var webQueryChild in webQueryChildren)
        {
            //Different methods are called here based on type (Different)
            statusReturn = _queryPermissions.DeleteWebQueryOutput(webQueryOutput);
            if (statusReturn.StatusCode.ToInt() == 500) return statusReturn;
        }
        return statusReturn;
    }

    private StatusReturn AddOrUpdate(IEnumerable<T> onlineReportChildren)
    {
        //AMC
        var webQueryOutputs = Mapper.Map<IEnumerable<OnlineReportOutput>, List<WebQueryOutput>>(onlineReportChildren);
        //Different
        return _queryPermissions.UpdateWebQueryOutputs(webQueryOutputs, false);
    }

    //These two methods could be made generic as well I imagine I just don't
    //know what to do with the calls
    protected List<WebQueryOutput> GetOutputs(int id, int type, string reportName)
    {
        var webQueryOutputs = new List<WebQueryOutput>();
        //Different
        _queryPermissions.GetWebQueryOutputs((QueryPermission.WebQueryTypes)type, ref webQueryOutputs, id, reportName);
        return webQueryOutputs;
    }

    protected IEnumerable<WebQueryInput> GetInputs(int id, int type, string reportName)
    {
        var webQueryInputs = new List<WebQueryInput>();
        //Different         
        _queryPermissions.GetWebQueryInputs((QueryPermission.WebQueryTypes)type, ref webQueryInputs, id, reportName);
        return webQueryInputs;
    }
}

As you can see, I am stuck on a few spots where there are different calls for the types. I am not sure how to handle the separate calls.

I am dependency-injecting the interface into my ApiControllers. I would rather my ApiControllers only know about the service / infrastructureObject that they are acting upon.

With more research, what if I used Contextual Binding with Ninject? It seems like that would give me one contract for both objects and I would still be able to keep my injections semi-clean.

Answer 1

A First Incremental Refactoring Suggestion

• The goal is to have working code without "major multiple organ replacement surgery" done all at once.
• Have a type parameter for each variant type.
• Keep all of your existing methods for now. Put all of them in the new class. Different signatures will keep things straight for the compiler.
• Define a delegate for each method that has different_queryPermissions calls, like Delete and AddAndUpdate. Hide the originals - make them protected. Make new public methods, same signatures as the originals of course, and all they do is call the delegate
• Set the delegates in the constructor. As the refactoring evolves I wonder if you're going to need a full-blown Abstract Factory.

//Not sure how to make Automapper calls generically (AMC)
    return Mapper.Map<List<WebQueryOutput>, List<OnlineReportOutput>>(webQueryOutputs);

More type parameters in the class:

public class OnlineReportChildService<T,U,X> : IOnlineReportChildService<T,U,X>
             where T : OnlineReportOutput
             where U : WebQueryOutput
             where X : OnlineReportOutput

A Delete Delegate

protected delegate StatusReturn DeleteDelegate(IEnumerable<T> In_n_Out_Burger)

EDIT

Now that Dependency Injection is in the discussions:

Abstract Factory and DI seem to be two sides of the same coin. And a quick internet search finds many discussions on Factory Patterns vs Dependency Injection. Indeed, StackOverflow has some.

In Dependency Injection in .NET the author warns "You'll need to exercise care not to use your DI Container as a Service Locator". The author calls Service Locator an anti-pattern (Other do not, particularly Martin Fowler who codified the pattern).

An extended quote:

If you thought of DI as a SERVICE LOCATOR - that is, a general-purpose Factory - this is something you need to unlearn. DI is the opposite of a SERVICE LOCATOR; it's a way to structure code so that we never have to imperatively ask for DEPENDENCIES. Rather, we force consumers to supply them.

some links

Service Locator is an Anti-Pattern

Inversion of Control Container and the Dependency Injection Pattern

Answer 2

You could also keep things simple and just use OO. Make an abstract class with two implementing subclasses. The parts that you tagged "AMC" or "Different" would be the abstract methods.