HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Responding to API requests with much complexity

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
muchrespondingwithrequestsapicomplexity

Problem

So having used the SE API multiple times, I like how all responses are in a Wrapper object, and for my own API design I figured it was a good method to follow.

Of course, I also wanted to abstract everything out so that common actions could be handled in a simple manner, so that I wouldn't need to write the same boilerplate code multiple times. (Wrap the entire response generation in a try/catch block then serialize exceptions to JSON, etc.)

Of course, since the inside of the try/catch block differs for each request, I needed a way to deal with that.

I got the idea to use a Request class which had a ProcessRequest() method that would call a DoWork() method which was implemented differently for each request.

Let's start with Request:

```
public abstract class Request
where T : IBaseModel
{
private ResponseType _responseType;

public Request(HttpContext context)
{
var responseTypeString = context.Request.QueryString["FileType"] ?? string.Empty;
_responseType = Utilities.Extensions.ResponseTypeExtensions.FromString(responseTypeString);
}

public string ProcessRequest()
{
try
{
var response = DoWork();
var responseWrapped = BuildWrapper(response);
var responseString = "";

switch (_responseType)
{
// For the Delimited Serializer types, serialize ONLY the Items. The Delimited Serializer doesn't support serializing graph objects like JSON and XML do.
case ResponseType.Csv:
responseString = DelimitedSerializer.CsvSerializer.Serialize(responseWrapped.Items);
break;
case ResponseType.Psv:
responseString = DelimitedSerializer.PsvSerializer.Serialize(responseWrapped.Items);
break;
case ResponseType.Tsv:
responseString = DelimitedSerializer.TsvSerializer.Serialize(responseWrap

Solution

Serialization

Where there's a switch there is usually an indication of a missing interface. In your case you could perfectly define a serializer interface:

public interface ISerializer
{
    public ResponseType ResponseType { get; }
    public string Serialize(object items);
}


and make the Request class extendable:

class Request
{
    // or a dictionary or whatever you like
    // you could also pass a collection to the constructor and make it immutable
    public List Serializers { get; set; }

    public string ProcessRequest()
    {
        return Serializers
            .Where(x => x.ResponseType == abc)
            .FirstOrDefault(x => x.Serialize(input));
    }
}


Constructor

public Request(HttpContext context)


We don't normally use public constructors in abstract classes. It should be protected.

Exception formatting

I don't like the exception formatting that the Request is currently responsible for. It doesn't look like it belongs there. The Request should take an exception formatter via DI. Formatting exceptions is a different matter and it doesn't fit well into a Request.

Naming

return Exception(e);


This is a very misleading and unclear name for a method. To understand what it does I had to first look at its source code. FormatException would be much better because its what it actually does.

IBaseModel


I'm not happy with this name either. It looks like it should rather be the name of the class RequestBase and not of the interface. For an interface a name like IRequest would be fine.

Code Snippets

public interface ISerializer
{
    public ResponseType ResponseType { get; }
    public string Serialize(object items);
}
class Request
{
    // or a dictionary or whatever you like
    // you could also pass a collection to the constructor and make it immutable
    public List<ISerializer> Serializers { get; set; }

    public string ProcessRequest()
    {
        return Serializers
            .Where(x => x.ResponseType == abc)
            .FirstOrDefault(x => x.Serialize(input));
    }
}
public Request(HttpContext context)
return Exception(e);

Context

StackExchange Code Review Q#136684, answer score: 6

Revisions (0)

No revisions yet.