C#•2y ago

✅ How can I improve this code?

It seems that I should change the way tags are stored. Or should I not?

public async Task<GetNotesByTagsQueryResult> Handle(GetNotesByTagsQuery query, CancellationToken ct)
    {
        var notes = new List<GetNotesByTagsQueryResultItem>();
        
        foreach (var excludedTag in query.ExcludedTags)
        {
            foreach (var includedTag in query.IncludedTags)
            {
                notes = await _db.Notes
                    .Where(n => n.Tags.Contains(includedTag) && !n.Tags.Contains(excludedTag))
                    .ProjectToType<GetNotesByTagsQueryResultItem>()
                    .Distinct()
                    .ToListAsync(ct);
            }
        }

        return new GetNotesByTagsQueryResult { Notes = notes };
    }

public async Task<GetNotesByTagsQueryResult> Handle(GetNotesByTagsQuery query, CancellationToken ct)
    {
        var notes = new List<GetNotesByTagsQueryResultItem>();
        
        foreach (var excludedTag in query.ExcludedTags)
        {
            foreach (var includedTag in query.IncludedTags)
            {
                notes = await _db.Notes
                    .Where(n => n.Tags.Contains(includedTag) && !n.Tags.Contains(excludedTag))
                    .ProjectToType<GetNotesByTagsQueryResultItem>()
                    .Distinct()
                    .ToListAsync(ct);
            }
        }

        return new GetNotesByTagsQueryResult { Notes = notes };
    }

16 Replies

Sossenbinder•2y ago

I don't really understand why you constantly reassign notes Your query already includes the filters to filter on the DB side That's perfect Can you maybe include all the filters at once from your loop? You currently have a lot of "wasted" code, you're potentially iterating a few times through your foreach, you query the DB for every loop, and constantly reassign notes, essentially throwing away the result from the last iteration So you should either check if you can include all filters in one or maybe only a few queries, or you need probably want to Add all the results to a List instead of throwing them away Right now you might query the DB e.g. 50 times, but essentially you only return the last query, making 49 queries absolutely unnecessary

demndevOP•2y ago

oh yes I hadn't thought about that. How can I make a DB query that includes and excludes specific tags? Query is:

public string IncludedTags { get; set;} = "tag1;tag2"
public string ExcludedTags { get; set;} = "tag3;tag4"

public string IncludedTags { get; set;} = "tag1;tag2"
public string ExcludedTags { get; set;} = "tag3;tag4"

I don't know what is the better way to get specific notes...

Sossenbinder•2y ago

.Where(n => n.Tags.Contains(includedTag) && !n.Tags.Contains(excludedTag))

.Where(n => n.Tags.Contains(includedTag) && !n.Tags.Contains(excludedTag))

That line already has the right idea, you can probably merge all IncludedTags into a combined .Where filter

demndevOP•2y ago

thanks

Sossenbinder•2y ago

Just as a side note, in case this might not work, you could technically also have db queries run concurrently (Just need to take care you don't overwhelm your DB with requests) by spawning the tasks all at once and awaiting them with Task.WhenAll

demndevOP•2y ago

I rewrote the code like this:

public async Task<GetNotesByTagsQueryResult> Handle(GetNotesByTagsQuery query, CancellationToken ct)
    {
        var notes = new List<GetNotesByTagsQueryResultItem>();

        var includedTagList = query.IncludedTags.Split(';');
        var excludedTagList = query.ExcludedTags.Split(';');
        
        notes.AddRange(await _db.Notes
            .Where(n => n.Tags.Split(';', StringSplitOptions.None)
                .Any(t => includedTagList.Any(includedTag => includedTag == t)))
            .Where(n => n.Tags.Split(';', StringSplitOptions.None)
                .Any(t => excludedTagList.All(excluded => excluded != t)))
            .ProjectToType<GetNotesByTagsQueryResultItem>()
            .Distinct()
            .ToListAsync(ct));

        return new GetNotesByTagsQueryResult { Notes = notes };
    }

public async Task<GetNotesByTagsQueryResult> Handle(GetNotesByTagsQuery query, CancellationToken ct)
    {
        var notes = new List<GetNotesByTagsQueryResultItem>();

        var includedTagList = query.IncludedTags.Split(';');
        var excludedTagList = query.ExcludedTags.Split(';');
        
        notes.AddRange(await _db.Notes
            .Where(n => n.Tags.Split(';', StringSplitOptions.None)
                .Any(t => includedTagList.Any(includedTag => includedTag == t)))
            .Where(n => n.Tags.Split(';', StringSplitOptions.None)
                .Any(t => excludedTagList.All(excluded => excluded != t)))
            .ProjectToType<GetNotesByTagsQueryResultItem>()
            .Distinct()
            .ToListAsync(ct));

        return new GetNotesByTagsQueryResult { Notes = notes };
    }

But now I got an error:

The LINQ expression 'DbSet<Note>()
    .Where(n => n.Tags.Split(
        separator: ;, 
        options: None)
        .Any(t => __includedTagList_0
            .Contains(t)))' could not be translated. Additional information: Translation of method 'string.Split' failed. If this method can be mapped to your custom function, see https://go.microsoft.com/fwlink/?linkid=2132413 for more information. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

The LINQ expression 'DbSet<Note>()
    .Where(n => n.Tags.Split(
        separator: ;, 
        options: None)
        .Any(t => __includedTagList_0
            .Contains(t)))' could not be translated. Additional information: Translation of method 'string.Split' failed. If this method can be mapped to your custom function, see https://go.microsoft.com/fwlink/?linkid=2132413 for more information. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

What can I do with the error? cc @Sossenbinder

Sossenbinder•2y ago

That means the LINQ expression you prepared can't be evaluated by EF Core (It tries to translate it into an SQL expression)

Sossenbinder•2y ago

Here is a list of supported operations: https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/ef/language-reference/supported-and-unsupported-linq-methods-linq-to-entities?redirectedfrom=MSDN

Supported and Unsupported LINQ Methods (LINQ to Entities) - ADO.NET

This article summarizes the standard query operators that are supported and unsupported in LINQ to Entities queries.

Sossenbinder•2y ago

So you would have to restructure a bit to make sure you can prepare something on client side which EF Core understands Oh, wait I'm an idiot haha The error even says what the issue is The Split operation is not supported

demndevOP•2y ago

how can I replace it?

Sossenbinder•2y ago

EF Core tries to evaluate the whole .Where to an SQL operation, but can't deal with Split You can probably execute the split on your client before you throw it into the IQueryable.Where(

demndevOP•2y ago

but how... how can I split it up before I want to access it in 'where'? doesn't access to the entity tag list appear during the query?

Sossenbinder•2y ago

Oh, I see, the split is part of your DB entity, my bad, I somehow thought this was part of your client side filters I can have a look later how you can circumvent it, gotta hop into a meeting now sadly

demndevOP•2y ago

thanks again

Sossenbinder•2y ago

Sorry for being a bit late But I guess regarding your issue: You might either go the route of structuring your identifiers in a better way on your sql table (So probably an additional table with respective mapping) or, you need to filter the data on your client Both not very satisfactory, but that's what your options are

demndevOP•2y ago

Thank you. I already did it the second way. But in the future I will make a separate table for tags for better performance.

Gaming

Programming

✅ How can I improve this code?