Open-closed principle for list result

Question

I'm practising how to create an open-closed principle with C# to return a list of info. but the problem is I'm getting an error in my main service class.

These are my sample codes:

I have this interface:

public interface ISelectInfo{
    bool Rule(string rule);
    IQueryable<FoodDto> ReturnSearchResult(string query);
}

I have this class that implements this interface.

public class Fruit : ISelectInfo {
    public bool Rule(string rule){ 
        return "fruit".Equals(rule); 
    }
    public IQueryable<FoodDto> ReturnSearchResult(string query){
        // returns a list of FoodDto
    }
}

And I have this main service class

public class SelectTypeOFoodService {
    private readonly IList<ISelectInfo> selectInfo;
    public SelectTypeOfFruitService (IList<ISelectInfo> selectInfo){
        this.selectInfo = selectInfo;
    }

    public async IEnumerable<FoodDto> SelectFood(string rule, string query){
        return await selectInfo.ToList().Where(x=>x.Rule(rule)).Select(x=>x.ReturnSearchResult(query)).AsQueryable().TolistAsync();
    }
}

I'm getting an error and red squiggle line on my return await selectInfo.ToList()...

What I'm trying to achieve it to return a list of FoodDto that based from Open-closed principle.

Here's the sample result of the red squiggle line that is showing Cannot convert expression type 'System.Collections.Generic.List<System.Linq.IQueryable<FoodDto>>' to return type 'System.Collections.Generic.IEnumerable<FoodDto>'

I hope somebody can help me.

`return await selectInfo.Where(x=>x.Rule(rule)).Select(x=>x.ReturnSearchResult(query)).AsQueryable().ToListAsync();` — Ehsan Sajjad, Dec 10 '18 at 11:09
This question has nothing to do with SOLID's OCP itself. It's all about linq/`IQueryable`/TAP. Also, you should provide the complete compiler error message and not just "I'm getting a red squiggle". — dymanoid, Dec 10 '18 at 11:13
@jsonGPPD OK, I' confused. What's the purpose of this code? And why use *Queryable* when the parameter is an IList? Finally, why use `await` and `ToListAsync()` when the data is in memory already? Asynchronous execution prevents blocking when performing IO operations. There are no blocking IO operations here — Panagiotis Kanavos, Dec 10 '18 at 11:17
@dymanoid I added additional info about the error on my question — jsonGPPD, Dec 10 '18 at 11:19
@jsonGPPD that doesn't help. The code is self-contradictory. What are you trying to do in the first place? What problem does that code try to solve? — Panagiotis Kanavos, Dec 10 '18 at 11:20
@PanagiotisKanavos that sample code aims to provide a autocomplete feature. Like for searching. — jsonGPPD, Dec 10 '18 at 11:20
So, my sample code only focusing to return a list from `Fruit` class, so in the future, let say, I want to do same thing for `Vegetable` I just going to extend by creating a new class and implements the `ISelectInfo` — jsonGPPD, Dec 10 '18 at 11:22
@jsonGPPD forget about that code. It confuses async with sync, enumerables with IQueryables. If you wanted to find the top N matches, a simple `words.Select(w=>w.someScoreFunction(input)).OrderByDescending().Take(3)` would work. If you wanted that to work in parallel you could just add `.AsParallel()` at the start. A *good* algorithm on the other hand would offer far better performance than brute-force scanning in parallel — Panagiotis Kanavos, Dec 10 '18 at 11:25
@jsonGPPD for example, if you wanted to use Levenshtein distance, a brute force implementation would be something like `words.AsParallel().OrderBy(w=>Levenshtein(w,input)).Take(3).ToArray();`. — Panagiotis Kanavos, Dec 10 '18 at 11:28
Please change the question description as it is misleading to Open Close Principle. — user2964808, Dec 22 '22 at 12:03

score 1 · Accepted Answer · answered Dec 10 '18 at 12:13

Alternatively, you can use an async on your IQueryable<FoodDto Check my refactor codes below.

public class Fruit : ISelectInfo {
    public bool Rule(string rule){ 
        return "fruit".Equals(rule); 
    }
    public async Task<IEnumerable<FoodDto>> ReturnSearchResult(string query){
        // returns a list of FoodDto
    }
}

On your service,

... 
public async Task<IEnumerable<FoodDto>> SelectFood(string rule, string query){
    return await selectInfo.FirstOrDefault(x=>x.Rule(rule)).Select(x=>x.ReturnSearchResult(query));
}
...

Also, don't forget to register your interfaces and class on your middleware or startup class.

Hope this help others as well.

Harald Coppoolse · Answer 2 · 2018-12-11T08:58:58.110

I always wonder why questioners give some code and say that the code does not work, without giving an exact requirement.

It seems to me, that you want to add an async function to your class SelectTypeOFoodService that takes two parameters as input:string rule and string query.

The output should be the ReturnSearchResult(query) of all items in this.selectInfos that have a true return value for this.selectInfos.Rule(rule)

Note: I took the liberty to pluralize the identifiers of your collections, as this would ease reading the answer.

It might be that you simplified your requirements, but to me it seems to me that you wouldn't need an async function for this.

IQueryable<FoodDto> QueryFood(string rule, string query)
{
    return this.selectInfos
        .Where(selectInfo => selectInfo.Rule(rule))
        .Select(selectInfo => selectInfo.ReturnSearchResult(query);
}

IEnumerable<FootDto> SelectFood(string rule, string query)
{
     return this.QueryFood(rule, query).AsEnumerable();
}

I chose use AsEnumerable instead of ToList, because apparently you want to return an IEnumerable. If your caller would use FirstOrDefault after calling your SelectFood it would have been a waste to convert all your thousand Foods into a list and then only use the first one.

If you really need to call ToList, for instance because you are disposing your queryable object, consider returning the List instead of IEnumerable. This will prevent users to call an extra ToList, causing a second list-ification of your items.

List<FoodDto> SelectFood(string rule, string query)
{
     return this.QueryFood(rule, query).ToList()
}

Now it can be meaningful to create an async version, because the ToList is a function that will actually perform the query, and if the query is to be performed by an awaitable process (like a database query, an internet fetch data, a file read), then it might be meaningful to create an async function:

Task<List<FoodDto>> SelectFoodAsync((string rule, string query)
{
     return this.QueryFood(rule, query).ToListAsync();
}

Once again: if there is nothing to await for, don't introduce async-await in the class, it will only make your code less efficient.

Sometimes, your classes have nothing to await for, but you do need to create an async function, for instance to implement an interface that returns a Task, or to mock functionality in a unit test to test functionality that uses async-await.

In that case, let the async function call the sync function and use Task.FromResult to compose the return value

Task<List<FoodDto>> SelectFoodAsync((string rule, string query)
{
    List<FoodDto> selectedFoods = this.SelectFoods(rule, query);
    return Task.FromResult(selectedFoods);
}

Open-closed principle for list result

2 Answers2