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

Accessing the Stack Exchange API

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

Problem

Part of a project I'm getting started on requires access to the Stack Exchange API for certain data, as a result I built a .NET implementation to interact with it.

The implementation is pretty simple, in my opinion. You can also find it on GitHub.

It features a main Configuration class, in which you put a lot of the API configuration information.

```
///
/// Represents a Stack Exchange API configuration for use with API requests.
///
public class Configuration
{
///
/// Represents the base endpoint for the Stack Exchange API url.
///
public const string ApiUrlBase = "{Protocol}://api.stackexchange.com/{Version}/";

///
/// This is the upper bound of the Page Size for most requests. Currently 100.
///
public const int MaxPageSize = 100;

///
/// The application API key. Can be null for anonymous requests.
///
public string Key { get; set; }

///
/// If true then the HTTPS protocol will be used, otherwise the HTTP protocol will be used. Defaults to true.
///
public bool UseHttps { get; set; } = true;

///
/// Determines what version of the API will be used. This should never be modified unless absolutely necessary. Defaults to 2.2.
///
public string Version { get; set; } = "2.2";

///
/// Returns the formatted with the provided parameters.
///
public string FormattedUrl => ApiUrlBase.Replace("{Protocol}", UseHttps ? "https" : "http").Replace("{Version}", Version);

///
/// Appends the current to the provided url.
///
/// The URL to append to. Should be the result of , then the API
///
public string AppendKey(string url) => string.IsNullOrWhiteSpace(Key) ? url : url + (url.Contains('?') ? '&' : '?') + "key=" + Key;

///
/// Returns the fully formatted URL for Stack Exchange API requests.
///
/// The fully filled making the request.
/// The formatted url.
public string GetFormattedUrl(IRequest request

Solution

Overall I like your code. It is understandable, (almost) easy to read and well structured. But I have some small pointers I would like to address.

-
In the Handler.SubmitRequest() method you have

var url = Configuration.GetFormattedUrl(request);


where I would preffer using the explicit type so one just looking at your code doesn't need to check that GetFormattedUrl() returns a string.

Another thing is the name of that method because it isn't only submitting a request but is processing the response as well. Maybe ProcessRequest would be a better name.

-
If one can use the Configuration class without setting any property he/she should have the possibility to use the Handler class without having to pass a Configuration to its constructor. You could add an parameterless constructor which passes a new Configuration() to the current constructor.

-
The FromattedEnpointUrl property of the InfoRequest looks odd to me. Whats the need for a dictionary if it contains only one key and value ? Why not let the StringExtensions.BuildQueryString() method have 2 string parameters ?

-
Some of the properties which aren't autoimplemented ones are hard to read because you placed both the getter and setter on the same line. This leads to the need to scroll to the right which is removing readability. Like this

public DateTime? OpenBetaDateTime { get { return DateTimeExtensions.FromEpoch(OpenBetaDate); } set { OpenBetaDate = DateTimeExtensions.ToEpoch(value); } }


IMO this would be better like so

public DateTime? OpenBetaDateTime 
{ 
    get { return DateTimeExtensions.FromEpoch(OpenBetaDate); } 
    set { OpenBetaDate = DateTimeExtensions.ToEpoch(value); } 
}


Btw, I hope the DateTimeExtensions.ToEpoch() method can handle nullable's well.

-
I don't like the AppendKey() method of the Configuration class either. Having 2 ternary expressions in a oneliner is IMO too much.

Code Snippets

var url = Configuration.GetFormattedUrl(request);
public DateTime? OpenBetaDateTime { get { return DateTimeExtensions.FromEpoch(OpenBetaDate); } set { OpenBetaDate = DateTimeExtensions.ToEpoch(value); } }
public DateTime? OpenBetaDateTime 
{ 
    get { return DateTimeExtensions.FromEpoch(OpenBetaDate); } 
    set { OpenBetaDate = DateTimeExtensions.ToEpoch(value); } 
}

Context

StackExchange Code Review Q#125330, answer score: 2

Revisions (0)

No revisions yet.