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

Library for the Summary API (used to get summaries of webpages)

Submitted by: @import:stackexchange-codereview··
0
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:

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

  • Parameters - the class name is too general, it should be SmmryParameters



  • 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.