1
\$\begingroup\$

I have the following code: It works completely as expected, but I feel there is a better way to refactor it out.

The flow is as follows:

  1. Call the end point with 1 page
  2. Get the total number of results.
  3. If total count of items more than itemsPerPage.
  4. Call the API more times till results from all pages are returned, join them together as single invoice list
  5. Append with initial request and send back.

Do I need GetAllInvoices function? Reason it is kept there is because I believe the responsibility of calling each pages of API does not belong to main function.

public async Task<IEnumerable<Invoice>> GetAllInvoicesAsync()
{
 const string endPoint = @"foo/{0}/invoices?pageNum={1}&itemsPerPage={2}";
 const int itemsPerPage = xxx;
 InvoiceCollection response = await _apiClient
 .GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, 1, itemsPerPage));
 if (response?.TotalCount > itemsPerPage)
 {
 var allInvoices = (await GetAllInvoices(endPoint,response.TotalCount, itemsPerPage))
 .SelectMany(i => i.Invoices?? Enumerable.Empty<Invoice>());
 response.Invoices = (response.Invoices ?? Enumerable.Empty<Invoice>()).Concat(allInvoices);
 }
 return response?.Invoices; 
}
private async Task<List<InvoiceCollection>> GetAllInvoices(string endPoint,int totalCount, int itemsPerPage)
{
 var totalPages = (int)Math.Ceiling(totalCount / (double)itemsPerPage);
 var tasks = new List<Task<InvoiceCollection>>();
 for (int currentPage = 2; currentPage<=totalPages;currentPage++)
 {
 tasks.Add(_apiClient
 .GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, currentPage, itemsPerPage)));
 }
 var response = new List<InvoiceCollection>(await Task.WhenAll(tasks));
 return response;
}
public class InvoiceCollection
 {
 [JsonProperty("foo")]
 public IEnumerable<Invoice> Invoices { get; set; }
 public int TotalCount { get; set; }
 }
asked Mar 2, 2020 at 14:25
\$\endgroup\$
2
  • \$\begingroup\$ Updated the title \$\endgroup\$ Commented Mar 2, 2020 at 14:56
  • 1
    \$\begingroup\$ I think @BCdotWEB means that the title should mention invoicing and that the current title can be asked within the body of the question. \$\endgroup\$ Commented Mar 2, 2020 at 15:30

1 Answer 1

1
\$\begingroup\$

When you have an assumption of getting a mix of return one or more values. You need to favor collection over single value return; because treating it as a collection would make things easier to deal with.

So, you can get rid of GetAllInvoices and adjust GetAllInvoicesAsync to something like this :

public async Task<IEnumerable<Invoice>> GetAllInvoicesAsync()
{
 const string endPoint = @"foo/{0}/invoices?pageNum={1}&itemsPerPage={2}";
 const int itemsPerPage = xxx;
 int pageNumber = 1; 
 int iterationCount = 0;
 var tasks = new List<Task<InvoiceCollection>>();
 var isFirstRound = true;
 do
 { 
 var response = await _apiClient.GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, pageNumber, itemsPerPage));
 if(isFirstRound) 
 { 
 iterationCount = response?.TotalCount <= itemsPerPage ? response?.TotalCount : (int) Math.Ceiling(totalPageItems / (double)itemsPerPage);
 isFirstRound = false;
 }
 tasks.Add(response);
 pageNumber++;
 } 
 while(pageNumber <= iterationCount)
 var invoiceResponseList = new List<InvoiceCollection>(await Task.WhenAll(tasks));
 var invoiceCollection = invoiceResponseList.SelectMany(i => i.Invoices ?? Enumerable.Empty<Invoice>());
 return invoiceCollection?.Invoices; 
}
answered Mar 4, 2020 at 18:13
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.