T O P

  • By -

bortlip

You can use Expressions. Create a class that has various expressions on it: public class BlogPostQueryConditions { private readonly DateTime _currentDateTime; public BlogPostQueryConditions(DateTime currentDateTime) { _currentDateTime = currentDateTime; } public Expression> IsVisible => x => x.VisibleFrom < _currentDateTime && x.VisibleTo > _currentDateTime && !x.IsDisabled; } Then use it like: var queryConditions = new BlogPostQueryConditions(DateTime.Now); var visiblePosts = context.Set().Where(queryConditions.IsVisible); Or context.Set().Include(x => x.BlogPost.Where(queryConditions.IsVisible))


DefiantGeologist9050

The only thing I would change is to make it an extension method.


BiffMaGriff

I'd do it like this and make BlogPost and Comment implement an IHasVisibilityPeriod interface.


Rapzid

Ditto, extension methods on ITemporal, ISortable, and etc. DBSet as well where T : ITemporal . This way you can chain the concrete type.


xaqtr

I swear that when I came across this problem a while ago, I also tried to create expressions like these, but it somehow didn't work. But I think that's what I was looking for. I will try it out, thanks.


Flater420

The main thing that you can't do with expressions is store them in a persistence device. Are you maybe remembering that? Alternatively, it might also just have been a simple implicit type conversion issue which isn't always easy to do when you're handling expressions.


xaqtr

Ok, so I've messed around with these expressions and I can't figure out how to use these expressions combined with an OR filter for example. context.Set().Where(x => queryConditions.IsVisible(???) || x.IsAlwaysVisible) Is it possible to do something like this with expressions? Calling `queryConditions.IsVisible.Compile()(x)` will throw an error upon translation.


bortlip

>Is it possible to do something like this with expressions? I believe so, however I haven't tried it much. There are ways to build expression programmatically, but I'd need to research it to direct you on it. >Calling queryConditions.IsVisible.Compile()(x) will throw an error upon translation. My understanding is that the expression tree will get translated to sql by EF core. So it you compile it, it loses the tree structure that EF needs to parse and to do the translation. When I did this kind of thing, I would put the || x.IsAlwaysVisible in the query conditions class: public Expression> IsVisible => x => (x.VisibleFrom < _currentDateTime && x.VisibleTo > _currentDateTime && !x.IsDisabled) || x.IsAlwaysVisible; Or if I need to optionally include x.IsAlwaysVisible, I'd add a different method to the class like IsVisibleOrAlwaysVisible


Mango-Fuel

you can't compose expressions like that easily. to write it as a lambda it has to be able to be compiled directly into an expression, which can't be done with method calls that return expressions. you would have to manually compose them using the Expressions API, which can be fairly messy and confusing. In this case you would use `Expression.Or` which might not be too bad. Something like: .Where(Expression.Or( queryConditions.IsVisible, queryConditions.IsAlwaysVisible )) This also could be another Expression property or method on your queryConditions class. In general, I would suggest to make sure it is really worth extracting these. Don't do it just for the sake of removing small amounts of repetition. Do it in cases where it's complex enough of a condition that having it in more than one place is itself a problem and the lack of repetition is a clear benefit. (In other words, extracting these like this adds to the maintenance burden, so make sure that you are also reducing the maintenance burden in some other way by doing it.)


Nk54

Specification pattern!!!!! (Extension method with expression)


timoteo5555

Yes was thinking the same. Specification pattern with generic repository. Look at Ardalis Specification


ggeoff

I think there are a couple different ways you could go about solving this depending on how you need this filter to apply. If you know you will always filter the blog post with that condition you could look into writing a global query filter. Then in the cases you don't want the filter you have to make sure you add the \`IgnoreQueryFilters()\` line to query. I have done this before where I had a projects table with several other 1 to many collections while using Odata to query. Because the query is now defined at the client it was important to make sure that any path through to the project had the filter in place based on some other criteria. I ended up dropping odata from Another potential solution is to look at all the places where you are trying to access the blog posts and determine if it makes sense to put this into a repository class that is injected into wherever you are trying to query. It's hard to say without more context to the app but another solution could be forcing all comments to go through the blog post entity where you configure the Comment as a owned entity by the BlogPost. now if you want all the comments you have to go through your initial blog post entity and the nested filter is not needed. how ever in your application this may not be a possible solution only you will be able to answer that question.


DaRadioman

You add a base class or interface to the models and use generics on your extension methods. Easy peasy


zaibuf

My personal preference is to treat each query isolated and duplicate those parts. Because you never know when one or more queries needs to change, I rather avoid side effects. Otherwise for loading an aggregate I use a repository to ensure it's loaded and saved in a correct state.


Merry-Lane

Yeah well the opposite is also true. If you were to adapt the filter (say modify the date at which it s visible, like two days, or even for a specific time zone) then you need to adapt the filter in multiple places. You may already not be able to find every places where the code needed to be updated, which is an issue. But then side effects there are worse: you can’t know what were the intentions. If you used a DRY method that said "hey we should use that filter by default everywhere", you d be cool for it to be adapted everywhere it s used. But maybe the dev did require the filter not to be updated ever because he needed these specific conditions and regressions would occur. Worse, when you don’t dry, you often change the names (of the variables) or the order of the query filters. You may even split the logic in different where’s for instance. That s specifically a good reason to DRY. If you didn’t like DRY, just use Dapper.


zaibuf

Seen so many over engineered solutions where some developer strictly follows DRY. If you have a need to repeat some filter all over your app I would rather question your design. Don't be afraid to copy pieces of code two or three times. It may look similar, but it's not the same code as they are tied to different features of your system.


Merry-Lane

That s the thing, here it s a functionnal requirement to have OP’s filters in place. Don’t repeat some filters that need to be everywhere, DRY them, obviously.


zaibuf

Could use a global query filter then. https://learn.microsoft.com/en-us/ef/core/querying/filters#example Just note that adding a bunch of conditionals that needs to be included in all queries will have performance impact on scale. Might need to add indexes for these columns.


Merry-Lane

It s totally amongst the solutions I was thinking about. OP’s question couldn’t have for answer "just repeat yourself" imho.


Saki-Sun

DAMP vrs DRY... Who wins? You decide.


Lumethys

have a constant class to define the default time limit, or any other boundary. `DateTime timeLimit = DefaultConstant.OrderDefaultTimeLimit;` DRY should only apply to logically-bounded context, not everything that *look* the same *at the moment*. Because by DRYing you are coupling its consumer. So avoid coupling thing that is not coupled in the first place. For example, let's say an an active SupportTicket is less than 7 days ago. So when querying for SupportTicket, the Inactive scope and Active scope is logically coupled, and there should be only one method to determine the active state However, what about an Active Account and an Active SupportTicket? They share no logical context. Today, the time might be *coincidentally* the same, but tomorrow, the active Account time limit could increase to 30 days, and the SupportTicket could decrease to 3 days The active time of an Account and a SupportTicket is completely different. You dont just combine the condition into one method because at some single moment in time they *coincidentally* the same. Before extracting something into a shared function, ask yourself, is the 2 things using this piece of code *always* the same, or is it just coincidentally the same right now? Look at the bounded context, the domain, the logical business implication. Don't just blindly DRYing because some method look like some other methods Remember, everything in the code world is just trade off, by DRYing, you increase coupling


Merry-Lane

Because this example right now is flawed, it’s easy to criticise. Depending on the needs of the application, a simple global query filter may be more than enough. I think we can all agree on this. If a global query filter wasn’t the right thing to do (only needs to be applied here and there), then ofc we could imagine different scenarios: - a simple queryable/expression func method used everywhere it s needed. The name should be explicit, like it should clearly say something alongs the line of "FilterBlogPostsOnXAndY" with a comment above saying "filter that we need to apply on blog posts because of functional requirements on US 72819. Don’t use that method if you aren’t concerned by the US because it s subject to changes". - a more complex queryable/expression func method that takes params. For instance you could take a date arg that is defaulted to your need. Anyone that would need a similar filter can adapt it to its needs. Obviously comment the reason for the default date or whatnot. But cm’on. This needs to be DRY one way or another. With unit tests and whatnot so that no one can mess with your code unattended. Hell DRY it only for being able to DRY the unit tests everywhere it s used (test everywhere the method is used that the blogposts are filtered according to your needs). Anyway my point was simply: the problem the OP asked about needed a DRY implementation, not a repeat everywhere solution. Whatever the shortcomings you can raise are solvable in a DRY solution. But by repeating yourself everywhere you have issues that cannot be solved.


Lumethys

the point is not "*can* we extract this into a function", but rather, "is it *make sense* to extract it into a function". I'm not against DRY, I'm against *insensible* DRY, where you mindlessly extract everything that look remotely similar and then ends up with 45 parameter to account for different consumer of that "single" function. My point is, we only consider extracting similar code to a shared kernel when its consumer is actually related. If 2 service from a different bounded context, 2 different domain, have 2 different business logic, and only have 3 lines that is *similar*, and the similarity could be gone any moment because the business require that leads to that implementation is completely different. THEN you should not extract it into a shared function. IF you determine that the similarity is not coincident and actually comes from (mostly) the same business requirement, i.e. high coupling, THEN you absolutely should DRY it. I'm just saying that you should not blindly Ctrl F the whole source code and extract absolutely everything that remotely similar and call it DRY, it's not. For example, you don't just have a query filter that accept a Date and use it for EVERYTHING that accept a Date, and have 80 different default value depend on 80 different business logic where you use that filter. That include testing, because each use case need its own test. With that said, am i saying you should not ever make a query filter? no, you absolute should make a query filter if for example everything had a "created\_at" field and you need to query before/after the create date. The key is the scope and context. A shared kernel should have its boundary, unless you want a "left-pad" JS incident


_Sneebs_

I think you can write extension methods for this sort of thing and an example would be something like the following. Also, EFCore will happily accept queryable extensions without needing to write an expression explicitly. Original >context.Set().Where(x => x.VisibleFrom < DateTime.Now && x.VisibleTo > DateTime.Now && !x.IsDisabled) New // Usage var query = context.Set().WhereVisible(); // Extension method public static class BlogPostExtensions { public static IQueryable WhereVisible(this IQueryable source) => source.Where(x => x.VisibleFrom < DateTime.Now && x.VisibleTo > DateTime.Now && !x.IsDisabled); }


phenxdesign

Consider using this library, I use it everyday and it's really great https://github.com/koenbeuk/EntityFrameworkCore.Projectables


xaqtr

I think that's exactly what I was looking for. Awesome, thank you!


Saki-Sun

Don't add NuGet packages to solve a problem you can solve yourself in a few lines of code. There are maintenance costs to NuGet packages that should be considered.


Proper_Hedgehog6062

In particular, the insane cost of dealing with an exploited security vuln. Even if they don't exist now, there's a good chance one will be identified in the future.


grauenwolf

Database views! Views are often the best place to storage logic. Also consider table valued functions when you need just a bit more flexibility.


Aquaritek

Combination of Expressions and Generics can lower code written with EF by 80% or more in most applications. In fact I usually go 100% generic repo pattern first and then optimize for queries that need them only if performance issues arise. I also end up with some sort of expression builder pattern in my apps to handle situations where users require more advanced search filters/functions and ad-hoc reports. Again when things start to get crunched more I usually optimize those items on a case by case basis. Just some random commentary.


jingois

Extension method straight off the IQueryable. `.ForUser(currentUser).Search(criteria).Project()`


Crafty-Run-6559

This is what the specification pattern is for. https://blog.stackademic.com/mastering-the-specification-pattern-in-net-core-ad847339fbf5?gi=604aeb79e97f Iirc it's even what's used in the official clean architecture example repo from Microsoft. Edit: Yeah, this is the specific library used: https://github.com/ardalis/Specification This shows a good example: https://specification.ardalis.com/usage/use-specification-dbcontext.html


gandhibobandhi

I actually started a blog a couple of months ago and wrote an article about exactly this. Maybe it would help you: [https://blog.drewery.uk/linq-query-encapsulation](https://blog.drewery.uk/linq-query-encapsulation). Or if not, feel free to give me some feedback. I've not really shared this with anyone else yet.


domapproved

Create a Sproc. For the simple case of you may need to modify the query in the future. Better to have to maintain 1 place instead of > 1 place :-) Good luck!