Client for many similar kinds of REST requests

Question

I have a class that makes calls to a Web API (Using RestSharp), which works fine but the code is super ugly. What would be the best way to refactor it? I thought of just doing a Facade pattern so all the calls that goes to cart will be in a separate class and order will be in an order class and so on.

All the methods looks very similar maybe I can extract something.

 public class ApiRestClient : IApiRestClient
    {

        private readonly RestClient _client;
        private readonly string _url = ConfigurationManager.AppSettings["webapibaseurl"];

        public ApiRestClient()
        {
            _client = new RestClient(_url);
        }

        public TokenDto Get(Guid id)
        {
            var request = new RestRequest("/cart/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<TokenDto>(response.Content);
        }

        public void SaveOrder(OrderDto order)
        {
            var request = new RestRequest("/order/", Method.POST) {RequestFormat = DataFormat.Json};
            request.AddObject(order);
            _client.Execute<TokenDto>(request);
        }

        public string GetLayout(int? id)
        {
            var request = new RestRequest("/customer/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);
            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<CustomerDto>(response.Content).Layout;
        }

        public void UpdateUser(UserDto userMap)
        {
            var request = new RestRequest("/user/", Method.POST) {RequestFormat = DataFormat.Json};
            request.AddObject(userMap);
            _client.Execute<UserDto>(request);
        }

        public IEnumerable<TokenDto> GetInvoices(Guid id)
        {
            var request = new RestRequest("/receipt/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<IEnumerable<TokenDto>>(response.Content);
        }

        public TokenDto GetInvoice(Guid id)
        {
            var request = new RestRequest("/receipt/invoice/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<TokenDto>(response.Content);
        }
    }

Answer 1

I don't see a reason for having a readonly _url in this class. In addition I don't see a reason to have a class level _url variable neither.
I would like to suggest having two constructors, one having a string url argument and one being argumentless. In this way it is easy without changing the AppSettings to have some flexibility.

Applying this would look like so

private readonly RestClient _client;

public ApiRestClient()
    : this(ConfigurationManager.AppSettings["webapibaseurl"])
{ }

public ApiRestClient(string url)
{
    _client = new RestClient(url);
}

It is not obvious from its name what the method public TokenDto Get(Guid id) will get. It seems from the resource that this would be a cart. Having a methodname, which makes it clear what the method is about, would be much better.

The GetLayout() method should be splitted into a GetCustomer() method (which you may need anyway) to only take the Layout of that returned CustomerDto.

The default Method for RestRequest is Method.Get so you could use the constructor which only takes string resource as a parameter.

The creation of this requests could be extracted to a method which takes a string and an object as parameter. Something along this lines

private RestRequest GetGetRequest(string resource, object value, string name)
{
    var request = new RestRequest(resource) {RequestFormat = DataFormat.Json};

    request.AddParameter(name, value, ParameterType.UrlSegment);

    return request;
}  

if you by any chance are using C# 6 you could use the nameof operator like so

private RestRequest GetGetRequest(string resource, object id)
{
    var request = new RestRequest(resource) {RequestFormat = DataFormat.Json};

    request.AddParameter(nameof(id), id, ParameterType.UrlSegment);

    return request;
}  

In the same way you could implement a GetPostRequest() method like so

private RestRequest GetPostRequest(string resource, object value)
{
    var request = new RestRequest(resource, Method.POST) { RequestFormat = DataFormat.Json };

    request.AddObject(value);

    return request;
}

You could also add a generic method ProcessRequest<T> like so

private T ProcessRequest<T>(RestRequest request)
{
    var response = _client.Execute(request);
    return JsonConvert.DeserializeObject<T>(response.Content);
}  

(I don't really know if I like this).

Applying the mentioned points will lead to

public class ApiRestClient : IApiRestClient
{

    private readonly RestClient _client;

    public ApiRestClient()
        : this(ConfigurationManager.AppSettings["webapibaseurl"])
    { }

    public ApiRestClient(string url)
    {
        _client = new RestClient(url);
    }

    private RestRequest GetGetRequest(string resource, object value, string name)
    {
        var request = new RestRequest(resource) { RequestFormat = DataFormat.Json };

        request.AddParameter(name, value, ParameterType.UrlSegment);

        return request;
    }

    private T ProcessRequest<T>(RestRequest request)
    {
        var response = _client.Execute(request);
        return JsonConvert.DeserializeObject<T>(response.Content);
    }

    public TokenDto GetCart(Guid id)
    {
        var request = GetGetRequest("/cart/{id}", id, "id");

        return ProcessRequest<TokenDto>(request);
    }

    public string GetLayout(int? id)
    {
        var customer = GetCustomer(id);

        return customer.Layout;
    }

    public CustomerDto GetCustomer(int? id)
    {
        var request = GetGetRequest("/customer/{id}", id, "id");

        return ProcessRequest<CustomerDto>(request);
    }

    public IEnumerable<TokenDto> GetInvoices(Guid id)
    {
        var request = GetGetRequest("/receipt/{id}", id, "id");

        return ProcessRequest<IEnumerable<TokenDto>>(request);
    }

    public TokenDto GetInvoice(Guid id)
    {
        var request = GetGetRequest("/receipt/invoice/{id}", id, "id");

        return ProcessRequest<TokenDto>(request);
    }

    private RestRequest GetPostRequest(string resource, object value)
    {
        var request = new RestRequest(resource, Method.POST) { RequestFormat = DataFormat.Json };

        request.AddObject(value);

        return request;
    }

    public void SaveOrder(OrderDto order)
    {
        var request = GetPostRequest("/order/", order);

        _client.Execute<TokenDto>(request);
    }

    public void UpdateUser(UserDto userMap)
    {
        var request = GetPostRequest("/user/", userMap);

        _client.Execute<UserDto>(request);
    }

}