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:
- Call the end point with 1 page
- Get the total number of results.
- If total count of items more than
itemsPerPage
. - Call the API more times till results from all pages are returned, join them together as single invoice list
- 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; }
}
-
\$\begingroup\$ Updated the title \$\endgroup\$Anjo– Anjo2020年03月02日 14:56:51 +00:00Commented 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\$pacmaninbw– pacmaninbw ♦2020年03月02日 15:30:31 +00:00Commented Mar 2, 2020 at 15:30
1 Answer 1
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;
}