What is the correct code organazing according to Clean Code and Refactoring with Inline method?-CodePudding

I have such a method

private async Task<List<ApiModel>> GetCollectionAsync(HttpClient httpClient,int categoryId, int pageNumber, string sessionId)
{
    var response = await RequestCollectionAsync(httpClient,categoryId, sessionId, (pageNumber - 1) * MaxItemsPerRequest);
    
    var items = response.Result.Items.ToList();
    
    if (HasMoreItems(response, pageNumber))
    {
        var collection = await GetCollectionAsync(httpClient, categoryId,   pageNumber, sessionId);
        if (collection.Count > 0)
        {
            items.AddRange(collection);
        }
    }
    
    return products;
}

As you can see here is a method HasMoreItems

private bool HasMoreItems(CollectionResponseModel collection, int pageNumber)
{
    var hasMoreItems = сollection?.Result?.Count > pageNumber * MaxItemsPerRequest;
    return hasMoreItems ;
}

This method is called only once.

The question is do we really need this method? Can we use in if statement just

if (сollection?.Result?.Count > pageNumber * MaxItemsPerRequest)

The second question is should I create a new method even on the LINQ select like this?

private List<OptionApiModel> GetOptionsGroupedByProperties(IList<ApiModel> cars)
{
    var options = cars
        .SelectMany(car => car.Options)
        .GroupBy(property => property.OptionId)
        .Select(group => group.FirstOrDefault())
        .ToList();
                
    return options;
}

Or better to use this select directly in the calling method?

CodePudding user response：

For a method that you use only once and having only a line, you can replace that line in the method that invoke your function:

    private async Task<List<ApiModel>> GetCollectionAsync(HttpClient httpClient,int categoryId, int pageNumber, string sessionId)
    {
        var response = await RequestCollectionAsync(httpClient,categoryId, sessionId, (pageNumber - 1) * MaxItemsPerRequest);

        var items = response.Result.Items.ToList();

        var hasMoreItems = сollection?.Result?.Count > pageNumber * MaxItemsPerRequest;
        if (hasMoreItems)
        {
            var collection = await GetCollectionAsync(httpClient, categoryId,   pageNumber, sessionId);
            if (collection.Count > 0)
            {
                items.AddRange(collection);
            }
        }

        return products;
    }

I maintain the use of a variable like hasMoreItems instead put the code in the if because the code is more readable. If you reuse HasMoreItem in more places, then it has sense the use of a method. But in this case, as you prefer.

For the second question ir more or les the same focus. You create a method to do something. If you are going to do the same thing many times, you can create a method and simplify that code with a single method call.

Even being little code or even a single line of code, it can make sense to encapsulate it in a method as it allows you to change that method and apply the changes everywhere it is used.