I've written a variety of libraries that query and work with data from APIs. While the classes and objects being passed around vary from project to project, I'm generating GET requests in the same manner in each project.
I have a basic method to generate a GET request:
private string GenerateWebRequest(string conn)
{
try
{
using (var webClient = new WebClient())
{
return webClient.DownloadString(conn);
}
}
catch (Exception e)
{
Console.WriteLine("There was a problem generating the web request to the API server.");
Console.WriteLine(e.StackTrace);
return string.Empty;
}
}
To make life easier and avoid code repetition, I create a Config
object in the primary constructor for each library, which looks something like:
public class Config
{
public Config()
{
localhostNearest = string.Empty;
localhostRoute = string.Empty;
localhostTrip = string.Empty;
processLocation = string.Empty;
workingDirectory = string.Empty;
datasource = string.Empty;
}
public string localhostNearest { get; set; }
public string localhostRoute { get; set; }
public string localhostTrip { get; set; }
public string processLocation { get; set; }
public string workingDirectory { get; set; }
public string datasource { get; set; }
}
And the instantiation of the Config
object:
Config cfg = new Config
{
localhostNearest = "http://localhost:5000/nearest/v1/driving/",
localhostRoute = "http://localhost:5000/route/v1/driving/",
localhostTrip = "http://localhost:5000/trip/v1/driving/",
processLocation = installationFolder + "\\osrm-routed.exe",
workingDirectory = workingFolder + @"data\"
};
The string conn
that gets passed as a parameter to the GenerateWebRequest()
method is generated in individual methods, dependent on what should be queried and then deserialized (each response is a JSON string). A sample conn
generation would be:
string response = GenerateWebRequest($"{_config.localhostNearest}{lat},{lon}?apiKey={key}");
which would come out as:
http://localhost:5000/route/v1/nearest/-3.3431053161621094,52.50671376947057?apiKey=XXXXX
The connection string is typically made up of three parts:
- Base URL. Such as http://localhost:5000/api/
- User query, which part of the API is being queried.
- API key appended to the end of the URL.
I feel like there is a cleaner way of doing this in each project, and some methods end up being overly verbose due to the creation of the URL.
Are there any recommendations that could be made to improve this?
1 Answer 1
Why are there Console.WriteLine calls? Use a logging library instead of binding this to a specific usage.
Also, returning string.Empty;
instead of throwing an exception seems a bit of a bad idea. You wouldn't know there's an issue unless you'd look at your logs.
Properties should be PascalCase: localhostNearest
, etc.
Why bother with this in the constructor?
localhostNearest = string.Empty;
Why can't it be null
?
Do not concatenate when working with paths:
processLocation = installationFolder + "\\osrm-routed.exe",
workingDirectory = workingFolder + @"data\"
Instead, use Path.Combine()
.
GenerateWebRequest()
does more than what its name implies.
What is conn
? Use descriptive names for variables, parameters, properties, etc.
There is a lot of repetition here:
localhostNearest = "http://localhost:5000/nearest/v1/driving/",
localhostRoute = "http://localhost:5000/route/v1/driving/",
localhostTrip = "http://localhost:5000/trip/v1/driving/",
If all URLs follow that pattern, why not work with a "template" -- e.g. $"http://localhost:5000/{type}/{version}/driving/"
-- and use string interpolation or String.Format()
?
Also, why are these hardcoded in your code? Shouldn't these URLs come from a .config file?
WebClient
, in which case it's recreated for each request. I could, for instance, instantiate it elsewhere and open/close it, but theusing
statement does that for me. \$\endgroup\$