patterncsharpMinor
Library for the Summary API (used to get summaries of webpages)
Viewed 0 times
theusedsummarygetforlibraryapisummarieswebpages
Problem
I was wondering if I could get some pointers on how I can improve my library, whether it be design pattern/naming convention/or any ideas on a better implementation.
GitHub
As it stands, I am using a builder pattern, so you'd go:
Then you would get the JSON which includes the summary of the webpage like so
Here is the code:
```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net.Http;
using System.Reflection;
namespace SummaryLib
{
public class Parameters
{
public string ApiKey { get; set; }
public string Url { get; set; }
public int? SentenceCount { get; set; }
public int? KeywordCount { get; set; }
public bool? IncludeQuotes { get; set; }
public bool? IncludeBreaks { get; set; }
}
public class Summary
{
private Parameters _parameters = new Parameters();
#region Builder
public Summary ApiKey(string _apikey)
{
_parameters.ApiKey = _apikey;
return this;
}
public Summary Url(string _url)
{
_parameters.Url = _url;
return this;
}
public Summary SentenceCount(int _sentencecount)
{
_parameters.SentenceCount = _sentencecount;
return this;
}
public Summary KeywordCount(int _keywordcount)
{
_parameters.KeywordCount = _keywordcount;
return this;
}
public Summary IncludeQuotes(bool _includequotes)
{
_parameters.IncludeQuotes = _includequotes;
return this;
}
public Summary IncludeBreaks(bool _includebreaks)
{
_parameters.IncludeBreaks = _includebreaks;
return this;
}
#endregion
public async Task GetJSON()
{
StringBuilder url = new StringBuilder($@"http://api.smmry.com");
url.Append(UrlBuilder(UrlParameters()));
using (var client = new HttpClient())
using (var respo
GitHub
As it stands, I am using a builder pattern, so you'd go:
var sum = new Summary();
sum.ApiKey(_apikey).Url(_url);Then you would get the JSON which includes the summary of the webpage like so
sum.GetJSON()Here is the code:
```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net.Http;
using System.Reflection;
namespace SummaryLib
{
public class Parameters
{
public string ApiKey { get; set; }
public string Url { get; set; }
public int? SentenceCount { get; set; }
public int? KeywordCount { get; set; }
public bool? IncludeQuotes { get; set; }
public bool? IncludeBreaks { get; set; }
}
public class Summary
{
private Parameters _parameters = new Parameters();
#region Builder
public Summary ApiKey(string _apikey)
{
_parameters.ApiKey = _apikey;
return this;
}
public Summary Url(string _url)
{
_parameters.Url = _url;
return this;
}
public Summary SentenceCount(int _sentencecount)
{
_parameters.SentenceCount = _sentencecount;
return this;
}
public Summary KeywordCount(int _keywordcount)
{
_parameters.KeywordCount = _keywordcount;
return this;
}
public Summary IncludeQuotes(bool _includequotes)
{
_parameters.IncludeQuotes = _includequotes;
return this;
}
public Summary IncludeBreaks(bool _includebreaks)
{
_parameters.IncludeBreaks = _includebreaks;
return this;
}
#endregion
public async Task GetJSON()
{
StringBuilder url = new StringBuilder($@"http://api.smmry.com");
url.Append(UrlBuilder(UrlParameters()));
using (var client = new HttpClient())
using (var respo
Solution
Improvables
Example
Here's how it could look like and let's with the attribute that will store the query string name of one parameter:
Then you decorate each property with this attribute:
At the same time you use a little compile trick with the
It also overrides the
Lastly you need a downloader to get the data:
Notice that the method's name is now
This is not a bullet-proof solution if you want to access the properties manually because it will throw if a key does not exist. You might want to use
Usage
Testing
In order to be able to test it you can create an interface for the downloader and use it to create another mock-downloader.
Parameters- the class name is too general, it should beSmmryParameters
Summary- this class is doing way to much. It not only builds the query string but also downloads the data. You need to separate it.
- Parameter names and query string names should be closer together, I mean use attributes to specify query string names. You also repeat each property name as a dictionary keys as a string. This is a really bad idea. You should at least use the
nameof.
Example
Here's how it could look like and let's with the attribute that will store the query string name of one parameter:
[AttributeUsage(AttributeTargets.Property)]
class SmmryParameterAttribute : Attribute
{
private readonly string _name;
public SmmryParameterAttribute(string name) { _name = name; }
public override string ToString() => _name;
}Then you decorate each property with this attribute:
public class SmmryParameters : Dictionary
{
[SmmryParameter("SM_API_KEY")]
public string ApiKey
{
get => (string)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_URL")]
public string Url
{
get => (string)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_LENGTH")]
public int? SentenceCount
{
get => (int?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_KEYWORD_COUNT")]
public int? KeywordCount
{
get => (int?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_QUOTE_AVOID")]
public bool? IncludeQuotes
{
get => (bool?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_WITH_BREAK")]
public bool? IncludeBreaks
{
get => (bool?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
private string GetSmmryParameterName([CallerMemberName] string propertyName = null)
{
return
GetType()
.GetProperty(propertyName)
.GetCustomAttribute()
.ToString();
}
public override string ToString() => $"?{string.Join("&", this.Select(item => $"{item.Key}={item.Value}&"))}";
}At the same time you use a little compile trick with the
CallerMemberName to get the property name and based on this the actual query string name that will be the key of the dictionary that the SmmryParameters class is derived from.It also overrides the
ToString method to generate the query string.Lastly you need a downloader to get the data:
public class SmmryDownloader
{
public async Task GetJsonAsync(SmmryParameters smmryParameters)
{
StringBuilder url = new StringBuilder($@"http://api.smmry.com");
url.Append(smmryParameters);
using (var client = new HttpClient())
using (var responsemessage = await client.GetAsync(url.ToString()))
using (var content = responsemessage.Content)
{
return await content.ReadAsStringAsync();
}
}
}Notice that the method's name is now
GetJsonAsync because by convention we add the Async suffix to async methods.This is not a bullet-proof solution if you want to access the properties manually because it will throw if a key does not exist. You might want to use
TryGetValue if you need to but it'll do for the current scenario otherwise you should implement the getters this way:get => TryGetValue(GetSmmryParameterName(), out object value) ? (string)value : default(string);Usage
var smmryParams = new SmmryParameters
{
ApiKey = "123",
Url = "https://en.wikipedia.org/wiki/Augustus"
};
var smmryDownloader = new SmmryDownloader();
json = smmryDownloader.GetJsonAsync(smmryParams);Testing
In order to be able to test it you can create an interface for the downloader and use it to create another mock-downloader.
Code Snippets
[AttributeUsage(AttributeTargets.Property)]
class SmmryParameterAttribute : Attribute
{
private readonly string _name;
public SmmryParameterAttribute(string name) { _name = name; }
public override string ToString() => _name;
}public class SmmryParameters : Dictionary<string, object>
{
[SmmryParameter("SM_API_KEY")]
public string ApiKey
{
get => (string)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_URL")]
public string Url
{
get => (string)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_LENGTH")]
public int? SentenceCount
{
get => (int?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_KEYWORD_COUNT")]
public int? KeywordCount
{
get => (int?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_QUOTE_AVOID")]
public bool? IncludeQuotes
{
get => (bool?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
[SmmryParameter("SM_WITH_BREAK")]
public bool? IncludeBreaks
{
get => (bool?)this[GetSmmryParameterName()];
set => this[GetSmmryParameterName()] = value;
}
private string GetSmmryParameterName([CallerMemberName] string propertyName = null)
{
return
GetType()
.GetProperty(propertyName)
.GetCustomAttribute<SmmryParameterAttribute>()
.ToString();
}
public override string ToString() => $"?{string.Join("&", this.Select(item => $"{item.Key}={item.Value}&"))}";
}public class SmmryDownloader
{
public async Task<string> GetJsonAsync(SmmryParameters smmryParameters)
{
StringBuilder url = new StringBuilder($@"http://api.smmry.com");
url.Append(smmryParameters);
using (var client = new HttpClient())
using (var responsemessage = await client.GetAsync(url.ToString()))
using (var content = responsemessage.Content)
{
return await content.ReadAsStringAsync();
}
}
}get => TryGetValue(GetSmmryParameterName(), out object value) ? (string)value : default(string);var smmryParams = new SmmryParameters
{
ApiKey = "123",
Url = "https://en.wikipedia.org/wiki/Augustus"
};
var smmryDownloader = new SmmryDownloader();
json = smmryDownloader.GetJsonAsync(smmryParams);Context
StackExchange Code Review Q#158347, answer score: 2
Revisions (0)
No revisions yet.