2
\$\begingroup\$

I have created a RESTful API that uses three other APIs with Visual Studio 2017 in .NET Core. It is the first time I have ever programmed in .NET so I am sure the code could use a lot of improvements. One of the biggest issues I have is with error handling.

Here are some questions that I have, but I am grateful for any help I can get. I just want to learn how to be a better .NET developer.

  1. How could the error handling be improved?
  2. Should the client classes have interfaces?
  3. Does the controller look good?

I have not included the models.

Controller

using System.Collections.Generic;
using Microsoft.AspNetCore.Mvc;
using MusicAPI.Models;
using System;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using static MusicAPI.ApiResponse;

namespace MusicAPI.Controllers
{
    [Route("api/[controller]")]
    public class MusicController : Controller
    {
        [HttpGet("error/{code}")]
        public IActionResult Error(int code)
        {
            return new ObjectResult(new ApiResponse(code));
        }

        // The API is called by accessing this method, by entering a mbid
        [HttpGet("{mbid}")]
        public async Task<IActionResult> GetByIdAsync(string mbid)
        {
            // The mbid needs to have the correct format and be the mbid of an artist, otherwise the call to the musicbrainz API will be unsucessful. 
            // Example valid mbid: 0383dadf-2a4e-4d10-a46a-e9e041da8eb3

            string query = mbid + "?&fmt=json&inc=url-rels+release-groups";
            MusicBrainzClient musicBrainzClient = new MusicBrainzClient();
            HttpResponseMessage response = await musicBrainzClient.GetAsync(query);

            if (response.IsSuccessStatusCode)
            {
                // The retrieved information will be stored in this object
                ArtistInfo artistInfo = new ArtistInfo();

                // Convert the content of the response to json
                var data = await response.Content.ReadAsStringAsync();
                dynamic json = JsonConvert.DeserializeObject(data);

                var getAlbums = GetAlbumsAsync(json); // Start task
                string band_name = GetBandName(json);

                WikipediaClient wikipediaClient = new WikipediaClient();
                var getWikipediaDescription = wikipediaClient.GetWikipediaDescriptionAsync(band_name); // Start task

                artistInfo.Mbid = mbid;
                artistInfo.Albums = await getAlbums;
                artistInfo.Description = await getWikipediaDescription;

                return new ObjectResult(new ApiOkResponse(artistInfo));
            }
            else
            {
                return NotFound(new ApiResponse((int)response.StatusCode, $"No entry found for mbid: {mbid}"));
            }
        }

        // Get id, title and cover image of every value in property "release-groups" of json
        public async Task<List<Album>> GetAlbumsAsync(JToken json)
        {
            List<Album> albums = new List<Album>();
            var tasks = new List<Task<Album>>();
            foreach (var release_group in json["release-groups"])
            {
                var getAlbum = GetAlbumAsync(release_group); // Start each task 
                tasks.Add(getAlbum);
            }

            // Perform the tasks in parallell until all the tasks are finished
            while (tasks.Count > 0)
            {
                // WhenAny can be used in a loop to solve problems that involve a small number of tasks.
                // Since the number of albums for an artist should be low, using WhenAny in a loop should not be a problem.
                var t = await Task.WhenAny(tasks); // Wait for any task to finish
                tasks.Remove(t);
                albums.Add(await t);
            }

            return albums;
        }


        public async Task<Album> GetAlbumAsync(dynamic release_group)
        {
            CoversClient coversClient = new CoversClient();
            Album album = new Album();
            string album_mbid = release_group["id"].ToString();

            album.Id = album_mbid;
            album.Title = release_group["title"].ToString();
            album.Image = await coversClient.GetAlbumCoverImageAsync(album_mbid);

            return album;
        }

        // The band name is used in the call to the wikipedia API
        // json needs to have the property "relations"
        public string GetBandName(JToken json)
        {
            string band_name = "";
            foreach (var relation in json["relations"])
            {
                if (relation["type"].ToString() == "wikipedia")
                {
                    var wikipedia_url = relation["url"]["resource"];

                    string url = JsonConvert.SerializeObject(wikipedia_url);
                    band_name = url.Substring(31); // Remove "https://en.wikipedia.org/wiki/
                    band_name = band_name.Remove(band_name.Length - 1); // Remove "
                }
            }

            return band_name;
        }
    }
}

Client #1

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;

namespace MusicAPI
{
    public class MusicBrainzClient
    {
        HttpClient musicBrainz_client = new HttpClient();

        public MusicBrainzClient() {
            musicBrainz_client.BaseAddress = new Uri("http://musicbrainz.org/ws/2/artist/"); // "We allow through 300 requests each second (on average), and decline (http 503) the rest." https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting
            musicBrainz_client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
            musicBrainz_client.DefaultRequestHeaders.Add("user-agent", "MusicAPI/0.1 (email)"); // Necessary to be allowed access to the musicbrainz API. More info: https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting
        }

        public async Task<HttpResponseMessage> GetAsync(string query)
        {
            return await musicBrainz_client.GetAsync(query);
        }
    }
}

Client #2

using MusicAPI.Models;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;

namespace MusicAPI
{
    public class CoversClient
    {
        HttpClient covers_client = new HttpClient();

        public CoversClient() {
            covers_client.BaseAddress = new Uri("http://coverartarchive.org/release-group/"); // "There are currently no rate limiting rules in place at http://coverartarchive.org" https://wiki.musicbrainz.org/Cover_Art_Archive/API
            covers_client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        }

        public async Task<string> GetAlbumCoverImageAsync(string cover_mbid)
        {
            string image = "";

            HttpResponseMessage response = await covers_client.GetAsync(cover_mbid);
            if (response.IsSuccessStatusCode)
            {
                // Convert the content of the response to json
                var cover_data = await response.Content.ReadAsStringAsync();
                dynamic cover_json = JsonConvert.DeserializeObject(cover_data);

                image = cover_json["images"][0]["image"];
            }
            else
            {
                image = "Album cover image not found";
            }

            return image;
        }
    }
}

Client #3

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;

namespace MusicAPI
{
    public class WikipediaClient
    {
        HttpClient client = new HttpClient();

        public WikipediaClient() {
            client.BaseAddress = new Uri("https://en.wikipedia.org/w/api.php"); // "There is no hard and fast limit on read requests" https://www.mediawiki.org/wiki/API:Etiquette
            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        }

        public async Task<string> GetWikipediaDescriptionAsync(string band_name)
        {
            string wikipedia_description = "";

            string query = "?action=query&format=json&prop=extracts&exintro=true&redirects=true&titles=" + band_name;
            HttpResponseMessage response = await client.GetAsync(query);

            if (response.IsSuccessStatusCode)
            {
                // Convert the content of the response to json
                var wikipedia_data = await response.Content.ReadAsStringAsync();
                dynamic wikipedia_json = (JObject)JsonConvert.DeserializeObject(wikipedia_data);

                JObject pages = (JObject)wikipedia_json["query"]["pages"];

                // The id is different for each wikipedia page, so can't do indexing as usual 
                foreach (JToken page in pages.Children())
                {
                    foreach (JToken info in page)
                    {
                        wikipedia_description = info["extract"].ToString();
                    }
                }
            }
            else
            {
                wikipedia_description = "Failed retrieving wikipedia description";
            }
            return wikipedia_description;
        }
    }
}

Error handling

using Newtonsoft.Json;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace MusicAPI
{
    public class ApiResponse
    {
        public int StatusCode { get; }

        [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
        public string Message { get; }

        public ApiResponse(int statusCode, string message = null)
        {
            StatusCode = statusCode;
            Message = message ?? GetDefaultMessageForStatusCode(statusCode);
        }

        private static string GetDefaultMessageForStatusCode(int statusCode)
        {
            switch (statusCode)
            {
                case 404:
                    return "Resource not found";
                case 500:
                    return "An unhandled error occurred";
                default:
                    return null;
            }
        }
    }
}

Also error handling

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace MusicAPI
{
    public class ApiOkResponse : ApiResponse
    {
        public object Result { get; }

        public ApiOkResponse(object result) : base(200)
        {
            Result = result;
        }
    }
}
\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.