1
\$\begingroup\$

I've created a simple API wrapper that I intend a few developers to utilize. I decided to follow an Fluent Interface methodology, similar to this.

public class SalesAPI
    {
        private HttpClient _client;
        private string _url;
        private List<string> CompanyNums;
        private List<string> CustBillToCodes;
        private List<string> CustShipToCodes;
        private List<string> CustPONum;
        private List<string> Warehouse;
        private List<string> ProdNum;
        private string StartDeliveryDate;
        private string EndDeliveryDate;
        private string StartShipDate;
        private string EndShipDate;
        private string StartPickDate;
        private string EndPickDate;
        private string Status;

        private JObject _request;


        public SalesAPI(string url)
        {
            _request = new JObject();
            _client = new HttpClient();
            _url = url;
            _request["appId"] = "APISALES";
            _request["command"] = "getSalesOrderDetails";
            _request["username"] = HttpContext.Current.User.Identity.GetShortADName();
        }
        public SalesAPI SetProdNum(List<string> prodNums)
        {
            ProdNum = prodNums;
            return this;
        }
        public SalesAPI SetProdNum(string prodNum)
        {
            ProdNum = new List<string>(new string[] { prodNum });
            return this;
        }

        public SalesAPI SetCompany(List<string> companies)
        {
            CompanyNums = companies;
            return this;
        }
        public SalesAPI SetCompany(string company)
        {
            CompanyNums = new List<string>(new string[] { company });
            return this;
        }
        public SalesAPI SetBillToCustCode(List<string> custCodes)
        {
            CustBillToCodes = custCodes;
            return this;
        }
        public SalesAPI SetBillToCustCode(string custCode)
        {
            CustBillToCodes = new List<string>(new string[] { custCode });
            return this;
        }
        public SalesAPI SetCustPONum(List<string> custPONums)
        {
            CustPONum = custPONums;
            return this;
        }
        public SalesAPI SetCustPONum(string custPONum)
        {
            CustPONum = new List<string>(new string[] { custPONum });
            return this;
        }
        public SalesAPI SetWarehouse(List<string> warehouses)
        {
            Warehouse = warehouses;
            return this;
        }
        public SalesAPI SetWarehouse(string warehouse)
        {
            Warehouse = new List<string>(new string[] { warehouse });
            return this;
        }
        public SalesAPI SetShipToCustCode(List<string> custCodes)
        {
            CustShipToCodes = custCodes;
            return this;
        }
        public SalesAPI SetShipToCustCode(string custCode)
        {
            CustShipToCodes = new List<string>() { custCode };
            return this;
        }
        public SalesAPI SetDeliveryDateRange(DateTime start, DateTime end)
        {
            StartDeliveryDate = start.ToPPROFormattedDate();
            EndDeliveryDate = end.ToPPROFormattedDate();
            return this;
        }
        public SalesAPI SetShipDateRange(DateTime start, DateTime end)
        {
            StartShipDate = start.ToPPROFormattedDate();
            EndShipDate = end.ToPPROFormattedDate();
            return this;
        }
        public SalesAPI SetPickDateRange(DateTime start, DateTime end)
        {
            StartPickDate = start.ToPPROFormattedDate();
            EndPickDate = end.ToPPROFormattedDate();
            return this;
        }

        public SalesAPI SetStatus(string status)
        {
            Status = status;
            return this;
        }
        private SalesAPI SetParameters()
        {
            JArray companyNumArray = new JArray(CompanyNums);
            JArray custBillToArray = new JArray(CustBillToCodes);
            JArray custShipToArray = new JArray(CustShipToCodes);
            JArray custPOArray = new JArray(CustPONum);
            JArray warehouseArray = new JArray(Warehouse);
            JArray prodNumArray = new JArray(ProdNum);

            if (CompanyNums != null) { _request["company"] = companyNumArray; }
            if (CustBillToCodes != null) { _request["custPOBillToID"] = custBillToArray; }
            if (CustShipToCodes != null) { _request["custShipToID"] = custShipToArray; }
            if (CustPONum != null) { _request["custPONum"] = custPOArray; }
            if (Warehouse != null) { _request["warehouse"] = warehouseArray; }

            if (StartDeliveryDate != null) { _request["startDeliveryDate"] = StartDeliveryDate; }
            if (EndDeliveryDate != null) { _request["endDeliveryDate"] = EndDeliveryDate; }

            if (StartShipDate != null) { _request["startShipDate"] = StartShipDate; }
            if (EndShipDate != null) { _request["endShipDate"] = EndShipDate; }

            if (StartPickDate != null) { _request["startPickDate"] = StartPickDate; }
            if (EndPickDate != null) { _request["endPickDate"] = EndPickDate; }

            if (Status != null) { _request["status"] = Status; }
            if (ProdNum != null) { _request["productNum"] = prodNumArray; }
            return this;
        }

        public async Task<RootSalesOrderObject> GetSalesOrderDetailsAsync()
        {
            SetParameters();
            var content = new StringContent(_request.ToString(Formatting.None), Encoding.UTF8, "application/json");
            var response = await _client.PostAsync(_url, content);
            var contents = await response.Content.ReadAsStringAsync();

            return JsonConvert.DeserializeObject<RootSalesOrderObject>(contents);
        }
    }

Here is how the api is consumed:

 var salesAPI = new SalesAPI("https://xxxxx.xxxxx.com/yyyyy/services");
            var companies = new List<string>()
            {
                "0002",
                "0007",
                "0009"
            };


           return await salesAPI
                             .SetCompany(companies)            
                             .SetDeliveryDateRange(DateTime.Now.AddDays(-3), DateTime.Now.AddDays(3))
                             .SetProdNum("17876")
                             .GetSalesOrderDetailsAsync();

Let me know what improvement you think could be made or if you think it is sufficient as is. Do you think this follows best practices and is relatively easy to understand for other developers who may need to use it?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

That SalesAPI is doing way too much. Which violates Single Responsibility Principle (SRP). The fluent interface also feels like over engineering but it can still be workable with some modifications to the current design.

My first suggestion would be to simplify the main class and follow Explicit Dependency Principle.

Methods and classes should explicitly require (typically through method parameters or constructor parameters) any collaborating objects they need in order to function correctly.

The following abstraction

public interface ISalesApi {
    Task<SalesOrderObject> GetSalesOrderDetailsAsync(SalesApiOptions options);
}

demonstrates this, with the SalesApiOptions argument being used to hold the necessary parameters for the request.

The SalesApiOptions will hold a collection of key-value pairs that represents the parameters to be sent to the API

public class SalesApiOptions : Dictionary<string, object> {

}

A series of extension methods can be used to implement the fluent interface on the options as parameters are set

public static class SalesApioptionsExtensions {
    public static SalesApiOptions SetProductNum(this SalesApiOptions options, List<string> productNumbers) {
        options["productNum"] = productNumbers;
        return options;
    }

    public static SalesApiOptions SetProductNum(this SalesApiOptions options, string productNumber) {
        options.SetProductNum(new List<string> { productNumber });
        return options;
    }

    public static SalesApiOptions SetCompany(this SalesApiOptions options, List<string> companies) {
        options["company"] = companies;
        return options;
    }

    public static SalesApiOptions SetCompany(this SalesApiOptions options, string company) {
        options.SetCompany(new List<string> { company });
        return options;
    }

    public static SalesApiOptions SetBillToCustCode(this SalesApiOptions options, List<string> custCodes) {
        options["custPOBillToID"] = custCodes;
        return options;
    }

    public static SalesApiOptions SetBillToCustCode(this SalesApiOptions options, string custCode) {
        options.SetBillToCustCode(new List<string> { custCode });
        return options;
    }

    public static SalesApiOptions SetCustPONum(this SalesApiOptions options, List<string> custPONums) {
        options["custPONum"] = custPONums;
        return options;
    }

    public static SalesApiOptions SetCustPONum(this SalesApiOptions options, string custPONum) {
        options.SetCustPONum(new List<string> { custPONum });
        return options;
    }

    public static SalesApiOptions SetWarehouse(this SalesApiOptions options, List<string> warehouses) {
        options["warehouse"] = warehouses;
        return options;
    }

    public static SalesApiOptions SetWarehouse(this SalesApiOptions options, string warehouse) {
        options.SetWarehouse(new List<string> { warehouse });
        return options;
    }

    public static SalesApiOptions SetShipToCustCode(this SalesApiOptions options, List<string> custCodes) {
        options["custShipToID"] = custCodes;
        return options;
    }

    public static SalesApiOptions SetShipToCustCode(this SalesApiOptions options, string custCode) {
        options.SetBillToCustCode(new List<string>() { custCode });
        return options;
    }

    public static SalesApiOptions SetDeliveryDateRange(this SalesApiOptions options, DateTime start, DateTime end) {
        options["startDeliveryDate"] = start.ToPPROFormattedDate();
        options["endDeliveryDate"] = end.ToPPROFormattedDate();
        return options;
    }

    public static SalesApiOptions SetShipDateRange(this SalesApiOptions options, DateTime start, DateTime end) {
        options["startShipDate"] = start.ToPPROFormattedDate();
        options["endShipDate"] = end.ToPPROFormattedDate();
        return options;
    }

    public static SalesApiOptions SetPickDateRange(this SalesApiOptions options, DateTime start, DateTime end) {
        options["startPickDate"] = start.ToPPROFormattedDate();
        options["endPickDate"] = end.ToPPROFormattedDate();
        return options;
    }

    public static SalesApiOptions SetStatus(this SalesApiOptions options, string status) {
        options["status"] = status;
        return options;
    }

    public static SalesApiOptions SetUsername(this SalesApiOptions options, string username) {
        options["username"] = username;
        return options;
    }
}

The key-value design of the SalesApiOptions still allows for other additional parameters to be added if needed.

options["someKey"] = "some value";

This simplifies the SalesApi to

public class SalesAPI : ISalesApi {
    private static HttpClient _client = new HttpClient();
    private string url;

    public SalesAPI(string url) {
        this.url = url;
    }

    public async Task<SalesOrderObject> GetSalesOrderDetailsAsync(SalesApiOptions options) {
        options["appId"] = "APISALES";
        options["command"] = "getSalesOrderDetails";
        // Parameters["username"] = HttpContext.Current.User.Identity.GetShortADName();

        var json = JsonConvert.SerializeObject(options);
        var content = new StringContent(json, Encoding.UTF8, "application/json");
        var response = await _client.PostAsync(url, content);
        var result = await response.Content.ReadAsAsync<SalesOrderObject>();
        return result;
    }
}

Note the removal of the tight coupling to HttpContext. I had a difficult time trying to figure out how to rework setting the user name without the tight coupling, but settled on an extension method

public static SalesApiOptions SetUsername(this SalesApiOptions options, string username) {
    options["username"] = username;
    return options;
}

and having the caller decide.

So with all that, calling the refactored API would look like

var salesAPI = new SalesAPI("https://xxxxx.xxxxx.com/yyyyy/services");
var companies = new List<string>()
{
    "0002",
    "0007",
    "0009"
};

var options = new SalesApiOptions()
    .SetCompany(companies)
    .SetDeliveryDateRange(DateTime.Now.AddDays(-3), DateTime.Now.AddDays(3))
    .SetProductNum("17876")
    .SetUsername(HttpContext.Current.User.Identity.GetShortADName());

return await salesAPI.GetSalesOrderDetailsAsync(options);
\$\endgroup\$
4
  • \$\begingroup\$ Nkosi, thanks for taking the time to respond to me! The reason I used the fluent interface is because I didn't want to have a method with a bunch of optional parameters. If you had to re-design this API, how would you get around having a bunch of optional parameters? Would you stick with the fluent API? \$\endgroup\$
    – jon.r
    Commented Aug 31, 2018 at 12:16
  • \$\begingroup\$ @Waragi that is why I chose the dictionary. That way I can include or exclude parameters. I would add some validation checks in the get method to make sure mandatory parameters were included. \$\endgroup\$
    – Nkosi
    Commented Aug 31, 2018 at 12:21
  • \$\begingroup\$ That's very true. Cool, thanks a lot! You taught me something new today. I'll refactor my code \$\endgroup\$
    – jon.r
    Commented Aug 31, 2018 at 12:23
  • \$\begingroup\$ I like the way you implemented that interface, forcing the developer to set the API parameters. Simple \$\endgroup\$
    – jon.r
    Commented Aug 31, 2018 at 13:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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