Interfaces that return values created by executing their callback parameters

Question

After searching for a while I did not find any answers to this question so here I go.

My general question is: Should interface methods return values produced by callbacks that are passed to them as parameters?

To demonstrate what I mean, I will give an example (the language in use is C# and the problem-domain is web but I think the question applies to many languages and domains).

Lets say you have a class that handles Http requests. It can be configured to accept parameters, http method and url and returns the response. The way I see it this method could either:

A) Provide a set of standard return types (maybe an error/success model and have a generic overload)

B) Accept a callback that gets the HttpResponse as parameter and lets the caller return whatever data he needs from it.

I realize that method A follows best practice principles but I think method B is applicable as well in some circumstances (for example you have a Services module that uses this Http handler class internally)

So my domain specific question is: Is it bad practice to have an interface method return a value produced by a callback in this specific situation?

To demonstrate my example with code:

public interface IWebRequestWrapper
{
   T DoRequest<T>(params... , Func<HttpResponse,T> callback);
} 

public class WebRequestWrapper : IWebRequestWrapper
{
  public T DoRequest<T>(params... , Func<HttpResponse,T> callback)
  {
     using (var response = httpClient.GetResponse(params))
     {
        return callback(response);
     }
  }
}

public class Client
{
   public SomeModel MakeServiceRequestX()
   {
      return iWebRequestWrapper.DoRequest(params, (i) => //construct your model through i and return it);
   }
}

Answer 1

The question basically boils to

1. Should any consumer of this interface need such degree of freedom as to be able to produce any value from HttpResponse
2. Will it lead to easy-to-maintain-design, or will it be a proverbial callback hell

Should any consumer of that interface need such degree of freedom as to be able to produce any value from HttpResponse

If we assume that we need to create many WebApiClients like

public class FooApiClient: IFooApiClient
{
    private readonly IWebRequestWrapper _webRequestWrapper;
    public FooApiClient(IWebRequestWrapper webRequestWrapper) =>
        _webRequestWrapper = webRequestWrapper ?? throw new ArgumentNullException();
    public FooBaseData GetBaseData(string id) =>    
        _webRequestWrapper.DoRequest(id, response => new FooBaseData...);
    public FooAdvancedData GetAdvancedData(string id) =>    
        _webRequestWrapper.DoRequest(id, response => new FooAdvancedData...);
}

Though unless we really need some custom HttpResponseProcessing, I'd rather expected that API to have swagger to allow those API clients to be generated automatically.

and our HttpResponse processing logic is at least slightly non-trivial (I mean not just get Json, parse if HttpStatusCode.Success/throw if anything else)

or if we have some generic(defined at run-time) endpoints

var result = _webRequestWrapper.DoRequest(
    TextBox1.Text, 
    response => Deserializer.Deserializer(GetType(ComboxBox1.Type), ...)

then it is a pretty reasonable design

1. It is composable (you can easily reuse it anywhere)
2. It makes consumer classes testable (that non-trivial HttpResponse processing logic that produces your actual domain objects)
   • Well, not with the real HttpResponse but I assume that it was not meant literally
   • In the worst case we will just have use some one-to-one wrapper MyHttpResponse that can be easily created/mocked.
3. It is concise and easy to use

Basically IEnumerable.Select for WebAPI calls.

Then what are the disadvantages?

1. Well, it is yet another abstraction layer. You gain some composability and testability, but also have to maintain one more class and interface. Overall not a bad deal, but still, unless you really need/will use that composability and testability, its a price paid for little-to-nothing.
2. It protects against someone forgetting to dispose WebResponse, but it doesn't protect the same person from forgetting that it will be disposed, so response should not/cannot be cached and value cannot be lazy initialized. Though you may say that it is obvious and the problem stands basically the same for inlined using (...HttpClient.Get, any abstraction makes things more difficult to spot.
   • Again, if we really want to avoid such risks, we can use some wrapper class that is created as a full snapshot of the received HttpResponse. Though it will lead to a small (though usually irrelevant) performance loss.

Will it lead to easy-to-maintain-design, or will it be a proverbial callback hell

As the first comment probably meant by its vehemence is that callbacks lead to callback hell. And that's true - those should be avoided at any cost.

But in this case it is a synchronous callback, not the dreaded asynchronous one from JavaScript.

Unless you nest another callbacks inside it, it is not much different from the regular Enumerable.Select that can be misused just as well.

---

Overall

As the second question is a non-issue, we are left the first one.

So if

1. There is very little logic in callbacks (no need to test it)
2. And/Or there are very few distinct callbacks
3. And/Or callbacks are often reused (code duplication)

Then it probably makes little sense to have such interface. Strongly typed API clients are the way to go (especially if they can be just generated with NSwagStudio or the likes).

But if none of those points apply (or apply very weakly), then such abstraction is a perfectly viable solution.

As always there are no absolute answers - everything depends on the exact situation.

about the testable part I really thought it gets harder to test this way instead of returning a specific dto

I meant cases when

our HttpResponse processing logic is at least slightly non-trivial (I mean not just get Json, parse if HttpStatusCode.Success/throw if anything else)

For example,

DoRequest("PeculiarApi", response =>
{
    if ((response.StatusCode == HttpStatusCode.Success) || 
        (response.StatusCode == HttpStatusCode.Conflict))
    {
        if (response.Headers.TryGet(Headers.ContentType, out var contentType) && 
            (contentType == ContentType.ApplicationJson))
        {
            return JsonConvert.Deserialize<TModel>(response.Content);
        }
        throw new Exception("No content type");
    }
    if (response.StatusCode == (HttpStatusCode)599) // Yes, that's what they return
    {
        return XmlConvert.Deserialize<T599Model>(response.Content).Errors[2];
    }
    ...
}

is a piece of non-trivial logic.

This logic can be easily tested through mocked IWebRequestWrapper.

Can it be tested with regular HttpClient? Yes, but that will require some boilerplate that is not needed with IWebRequestWrapper.

By itself it is not worth much (not a deal-breaker), but if we already have to use IWebRequestWrapper it is at least a minor simplification.