patterncsharpMinor
The thing that makes API requests
Viewed 0 times
themakesthatrequestsapithing
Problem
I'm creating a thing that does things. Part of the things the thing does is querying an API for information about movies and TV shows. The API allows me to query for changes to their data so I will first iterate over every entry they have (IDs are numerically represented), store its information locally and then periodically update the database with information I get from the Changes API.
The part of the thing you see here is how I create requests and how each API call will be implemented. The setup is simple: since almost all kinds of requests are the same, I just create subclasses of a general request that implement request-specific behaviour.
Advantages:
Disadvantages:
Thoughts:
-
-
Is this a scalable approach? I've thought about things that might hinder me in the future but since each request can take an authorization header and
-
I'm using a
Type hierarchy:
Code
API interface
```
namespace TMDbWrapper
{
// ReSharper disable once InconsistentNaming
public class TMDbApi
{
private readonly string _apikey;
private const string BaseUrl = "https://api.themoviedb.org/3/";
public TMDbApi(string apikey)
{
_apikey = apikey;
}
priva
The part of the thing you see here is how I create requests and how each API call will be implemented. The setup is simple: since almost all kinds of requests are the same, I just create subclasses of a general request that implement request-specific behaviour.
Advantages:
- GET, POST, PUT, DELETE, etc are all trivially implemented
- Minimal code duplication
- Creating a new API call takes less time than removing a booger
Disadvantages:
- Some requests have information they can't use (
GetRequestis being passedHttpContent)
Thoughts:
-
HttpClient is made for re-use but right now I create a new one for every request. I could store it as a static field in AbstractRequest but I'm not entirely sure if this would be a good approach.-
Is this a scalable approach? I've thought about things that might hinder me in the future but since each request can take an authorization header and
HttpContent, I think most common API call scenarios have been covered.-
I'm using a
Response class to store information about the response which is right now just the HTTP status code. I'm still debating what combination of "return the data", "return the response" and "write to logfile" to use as to how I handle failed requests.Type hierarchy:
Code
API interface
```
namespace TMDbWrapper
{
// ReSharper disable once InconsistentNaming
public class TMDbApi
{
private readonly string _apikey;
private const string BaseUrl = "https://api.themoviedb.org/3/";
public TMDbApi(string apikey)
{
_apikey = apikey;
}
priva
Solution
Instead of having a private
Changing to
Otherwise you code looks clean and well structured.
CreateClient() method you should consider to make it protected virtual if you would ever have the need to use headers different to "application/json". Changing to
protected virtual forInternalExecuteRequestAsync()`should be done too.
You have small code duplication in the TMDbApi class.
The GetTvGenres() and the GetMovieGenres() are doing the same thing but with a different url. This should be extracted to a separate method.
Instead of always adding the strings BaseUrl + apiRequest + "?api_key=" + _apikey; in the GetUrl() method, you should consider to either change _apikey to _apikeyOperator which you initialize in the constructor to _apikeyOperator = "?api_key=" + apikey;
private string GetUrl(string apiRequest)
{
return BaseUrl + apiRequest + _apikeyOperator;
}
or to change BaseUrl from const to readonly initialized to BaseUrl = "https://api.themoviedb.org/3/{0}?api_key=" + apikey; and then change the GetUrl() method to using String.Format().
private string GetUrl(string apiRequest)
{
return String.Format(BaseUrl, apiRequest);
}
You should consider to break the return of the GetKeywords() into 2 lines to grasp it at first glance without scrolling.
Because you use apiRequest you should stick to camelCase casing for apikey too.
I would stick to creating the HttpClient for every request because by using the using` keyword you ensure that all ressources are freed. Otherwise you code looks clean and well structured.
Code Snippets
private string GetUrl(string apiRequest)
{
return BaseUrl + apiRequest + _apikeyOperator;
}private string GetUrl(string apiRequest)
{
return String.Format(BaseUrl, apiRequest);
}Context
StackExchange Code Review Q#79972, answer score: 7
Revisions (0)
No revisions yet.