patterncsharpMinor
Responding to API requests with much complexity
Viewed 0 times
muchrespondingwithrequestsapicomplexity
Problem
So having used the SE API multiple times, I like how all responses are in a
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
Of course, since the inside of the
I got the idea to use a
Let's start with
```
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
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:
and make the
Constructor
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
Naming
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.
I'm not happy with this name either. It looks like it should rather be the name of the class
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.IBaseModelI'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.