2

Let's take an example of a generic API endpoint that looks as follows:

https://api.imgur.com/3/gallery/{section}/{sort}/{window}/{showViral}/{page}

Conditions on this endpoint are as follows:

  1. Arguments are optional. Default values will be used if not provided.
  2. All the earlier arguments must be provided in order to use an argument. So in this example, if I were to get posts of page no 2, I must provide section, sort, window and showViral values.

I am trying to create a C# wrapper around such endpoints. My method looks somewhat as follows:

public static async Task<List<Image>> GetGallery(Section? section = null, Sort? sort = null, Window? window = null, bool? showViral = null, int? page = null)
{
    string uri = "gallery";
    if (section != null)
    {
        uri += "/" + section.ToString().ToLower();
        if(sort != null)
        {
            uri += "/" + sort.ToString().ToLower();
            if(window != null)
            {
                uri += "/" + window.ToString().ToLower();
                if (showViral != null)
                {
                    uri += "/" + showViral.ToString();
                    if (page != null)
                    {
                        uri += "/" + page;
                    }
                }
            }
        }
    }
    JObject response = await NetworkHelper.ExecuteRequest(uri);
    return response["data"].ToObject<List<Image>>();
}

I'm not really happy with this implementation. This code looks very ugly. Any thoughts on how I can improve this?

  • Is this on topic here? Sounds like a job for Code Review SE. – RubberDuck Sep 13 '15 at 1:18
  • @RubberDuck Not sure, but question seems inline with the other questions here. Anyhow, I'm not really asking for review. I'm asking for entirely different solutions. – akshay2000 Sep 13 '15 at 5:01
3

A simple solution using LINQ:

string CreatePath(params string[] segments)
{
    var includedSegments = segments.TakeWhile(segment => segment != null));
    return String.Join("/", includedSegments);
}

Then you just need to pass your string segment arguments in order, converted to string. Alternatively you could do the string conversion inside:

string CreatePath(params object[] segments)
{
    var includedSegments = segments.TakeWhile(segment => segment != null)).Select(segment => segment.ToString());
    return String.Join("/", includedSegments);
}
2

You can write a fluent solution. It needs some interfaces, but the usage for a client is very simple and guided, since autocompletion will automatically suggest two possibilities at each point: Images or the subcriteria, e.g. Sort(...);

So for the client, the usage would be like this:

var g = // get gallerygetter from somewhere 

// get all images
var images = g.Get().Images;

// subsets
var images = g.Get().Section(section).Sort(sort).Images;

var images = g.Get()
    .Section(section)
    .Sort(sort)
    .Window(window)
    .ShowViral(showViral)
    .Page(page)
    .Images;

Now of course, in order to support such a nice use, you need some interfaces. But you can implement all those in a single class.

public interface IGallerySectionSortWindowShowViralPage : IGallery
{ 
    IGallery Page(int x);
}

public interface IGallerySectionSortWindowShowViral : IGallery
{
    IGallerySectionSortWindowShowViralPage ShowViral(bool x);
}

public interface IGallerySectionSortWindow : IGallery
{
    IGallerySectionSortWindowShowViral Window(Window w);
}

public interface IGallerySectionSort : IGallery
{
    IGallerySectionSortWindow Sort(Sort s);
}

public interface IGallerySection : IGallery
{
    IGallerySectionSort Section(Section s);
}

public IGallery
{
    Task<List<Image>> Images { get; }
}

public IGalleryGetter : IGallery
{
    IGallerySection Get();
}

public Gallery 
    : IGallery
    , IGalleryGetter
    , IGallerySection
    , IGallerySectionSort
    , IGallerySectionSortWindow
    , IGallerySectionSortWindowShowViral 
    , IGallerySectionSortWindowShowViralPage
{
    private string uri = "gallery";

    public Task<List<Image>> Images 
    {
        JObject response = await NetworkHelper.ExecuteRequest(uri);
        return response["data"].ToObject<List<Image>>();
    }

    public IGallerySection Get()
    {
        return this;
    }

    public IGallerySectionSort Section(Section section)
    {
        uri += "/" + section.ToString().ToLower();
        return this;
    }

    public IGallerySectionSortWindow Sort(Sort sort)
    {
        uri += "/" + sort.ToString().ToLower();
        return this;
    }

    public IGallerySectionSortWindowShowViral Window(Window window)
    {
        uri += "/" + window.ToString().ToLower();
        return this;
    }

    public IGallerySectionSortWindowShowViralPage ShowViral(bool show_viral)
    {
        uri += "/" + show_viral.ToString().ToLower();
        return this;
    }

    public IGallery Page(int page)
    {
        uri += "/" + page.ToString().ToLower();
        return this;
    } 

}
  • This is really innovative. I hadn't thought about this. One problem that I see is the service could have many different endpoints for different things and I'll end up with so many interfaces. But seriously, great solution. – akshay2000 Sep 11 '15 at 16:16
2

You can improve this by separate the path joining method.

public static string BuildPath(params object[] paths) {
    string result = "";
    if (paths != null) {        
        foreach(var path in paths) {
            if (path == null) break;
            result += "/" + path;
        }
    }
    return result;
}

Then your implementation will make more sense, because it is one-liner

string uri = BuildPath("gallery", section, sort, window, showViral, page);

I did this in a fiddler here: https://dotnetfiddle.net/w9n0wg. Kindly noted that this is mimicking of your code behaviour, and does not do URL path encoding at all.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.