C# : Is there a way for a method with all optional parameters to force at least one parameter-CodePudding

Recently I have made a function for which all parameters are optional, but at the same time I need at least one parameter to be passed, otherwise the function doesn't do anything.

public async Task<discogsResult.DiscogsSearchResults?> Search(string? query = null, string? title = null, string? release_title = null, string credit? = null, string? artist = null, [...])
{
    if (string.IsNullOrEmpty(query) &&
        string.IsNullOrEmpty(title) &&
        string.IsNullOrEmpty(release_title) &&
        string.IsNullOrEmpty(credit) &&
        string.IsNullOrEmpty(artist) &&
        [...])
    {
        return null;
    }

    var discogsClient = new DiscogsClient.DiscogsClient(tokenInformation);

    var discogsSearch = new DiscogsSearch()
    {
        query = query,
        type = type,
        title = title,
        release_title = release_title,
        credit = credit,
        [...]
    };

    return await discogsClient.SearchAsync(discogsSearch);
}

I currently deal with it (check all parameters for null) and return empty if it is the case. But I would like to force the function caller to pass at least one because currently the function can be called with none.

Is there a way for the parameters to do that in c# ?

CodePudding user response：

One approach is to define multiple methods, one for each non-null case, and then call a private method that actually implements the method.

For example (I've changed the return type to Task<string?> rather than Task<discogsResult.DiscogsSearchResults?> to simplify):

public Task<string?> SearchQuery(string query, string? title = null, string? releaseTitle = null, string? credit = null, string? artist = null)
{
    return searchQuery(query ?? throw new ArgumentNullException(nameof(query)), title, releaseTitle, credit, artist);
}

public Task<string?> SearchTitle(string title, string? query = null, string? releaseTitle = null, string? credit = null, string? artist = null)
{
    return searchQuery(query, title ?? throw new ArgumentNullException(nameof(title)), releaseTitle, credit, artist);
}

public Task<string?> SearchReleaseTitle(string releaseTitle, string? query, string? title, string? credit = null, string? artist = null)
{
    return searchQuery(query, title, releaseTitle ?? throw new ArgumentNullException(nameof(releaseTitle)), credit, artist);
}

public Task<string?> SearchCredit(string credit, string? query = null, string? title = null, string? releaseTitle = null, string? artist = null)
{
    return searchQuery(query, title, releaseTitle, credit ?? throw new ArgumentNullException(nameof(credit)), artist);
}

public Task<string?> SearchArtist(string artist, string? query = null, string? title = null, string? releaseTitle = null, string? credit = null)
{
    return searchQuery(query, title, releaseTitle, credit, artist ?? throw new ArgumentNullException(nameof(artist)));
}

Task<string?> searchQuery(string? query, string? title, string? releaseTitle, string? credit, string? artist)
{
    // Actual implementation and any further parameter validation goes here.
    return Task.FromResult<string?>(null);
}

Some advantages of this approach:

Code cannot call the methods with an incorrect null parameter without the compiler issuing a warning.
The name of the method explicitly states which parameter cannot be null.

Some disadvantages of this approach:

Far more coding (one method per non-null parameter).
The order of the parameters is subtly different for each overload which could cause user confusion.

CodePudding user response：

No, you are already doing what most people would likely do here, and validating inside the function call. Another alternative would be to let the caller know they passed in bad information with a return value or exception.

You could move all the parameters into a class and have various constructors to ensure a complete model is being passed in though. For example:

public class SearchModel
{
    // Stop people making a class without validation
    private SearchModel()
    {
    }

    public string? Query { get; set; } = null;
    public string? Title { get; set; } = null;
    public string? ReleaseTitle { get; set; } = null;
    public string? Credit { get; set; } = null;
    public string? Artist { get; set; } = null;

    // If people are regularly going to search by query or title, then
    // you can make simple methods to do that, for example:

    public static SearchModel SearchByQuery(string query)
    {
        if (string.IsNullOrEmpty(query)) throw new ArgumentNullException();
        return new SearchModel { Query = query };
    }

    public static SearchModel SearchByTitle(string title)
    {
        if (string.IsNullOrEmpty(title)) throw new ArgumentNullException();
        return new SearchModel { Title = title };
    }

    // Other common search options...


    // And a generic one for all other options
    public static SearchModel SearchByAll(string? query = null, string? title = null, 
        string? releaseTitle = null, string? credit = null, string? artist = null)
    {
        if (string.IsNullOrEmpty(query) &&
            string.IsNullOrEmpty(title) &&
            string.IsNullOrEmpty(releaseTitle) &&
            string.IsNullOrEmpty(credit) &&
            string.IsNullOrEmpty(artist))
        {
            throw new ArgumentNullException();
        }
        
        return new SearchModel
        {
            Query = query,
            Title = title,
            //... etc.
        }
    }
}

And fix your method:

public async Task<discogsResult.DiscogsSearchResults?> Search(SearchModel model)

And now you can do this:

var model = SearchModel.SearchByTitle("my title");
var searchResult = await Search(model);

CodePudding user response：

When having all strings as single incoming parameters, and at least one has to be set, you will have to check this condition. However, depending on how performance critical it is, you could simplify these checks. Example: You can merge all in a array, and use Linq to check the condition. That would be easier to extend later:

if (new[] { query, title, release_title, credit, artist }
                  .All(x => string.IsNullOrEmpty(x)))
{
     return null;
}

Side note from me: I'd prefer to use the DiscogsSearch class to provide a "IsValid"-Property/Function to verify it contains valid values to perform a search.