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?

Improve this question
2
  • Is this on topic here? Sounds like a job for Code Review SE. Commented Sep 13, 2015 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. Commented Sep 13, 2015 at 5:01

3 Answers 3

Reset to default
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); }
Improve this answer
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; } }
Improve this answer
1
  • 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. Commented Sep 11, 2015 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.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.