Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request to chain And and Or Conditions in Where Clause #52

Closed
fingers10 opened this issue Oct 20, 2020 · 4 comments
Closed

Feature request to chain And and Or Conditions in Where Clause #52

fingers10 opened this issue Oct 20, 2020 · 4 comments

Comments

@fingers10
Copy link

It would be great if we can add dynamic where clause chaining into specification. For example, if we need to dynamically include search values in where clause based on condition or to support advanced search scenarios; can something like this would make sense?

public class ItemFilterSpecification : Specification<ItemEntity>
{
    public ItemFilterSpecification(string? itemName, string? itemCode) : base()
    {
        if (!string.IsNullOrWhiteSpace(itemName))
        {
            Query.Where(i => i.Name == itemName);
        }

        if (!string.IsNullOrWhiteSpace(itemCode))
        {
            Query.Where(i => i.Code== itemCode); // WhereAnd or Where can be defaulted to And search
            Query.WhereOr(i => i.Code== itemCode); // WhereOr to include Or conditions in search 
        }

        // Till this point if both values are not null then query should be composed like 
        // Query.Where(i => i.Name == itemName && i.Code == itemCode);
        // For Or search
        // Query.Where(i => i.Name == itemName || i.Code == itemCode);
    }
}

This would add great value to the package. And help to build query for advanced search or dynamic search

@fiseni
Copy link
Collaborator

fiseni commented Oct 20, 2020

Hi @fingers10,

This feature is useful, but it's out of the scope of the specifications (as implemented in this package).
In order to implement it, we need to manipulate the expressions that the user enters. We should keep track of all the conditions and then re-constructs the expressions. This will increase the complexity of the package significantly. In the current state, we just hold the expressions, and pass them to the ORM as they are.
It's not about if it's hard to implement or not (it can be done). It's the fact that the package suddenly will become something that users have to master it. Personally, I prefer it to remain useful and yet not to be struggle to use it. @ardalis any thoughts on this?

Btw, you can accomplish the functionality you want with one additional condition. It's not elegant, but it's simple.

public class ItemFilterSpecification : Specification<ItemEntity>
{
    public ItemFilterSpecification(string? itemName, string? itemCode) : base()
    {
        if (!string.IsNullOrWhiteSpace(itemName) && !string.IsNullOrWhiteSpace(itemCode))
        {
            Query.Where(c => c.Name == itemName || c.Code == itemCode);
        }
        else if(!string.IsNullOrWhiteSpace(itemName))
        {
            Query.Where(i => i.Name == itemName);
        }
        else if(!string.IsNullOrWhiteSpace(itemCode))
        {
            Query.Where(i => i.Code == itemCode);
        }
    }
}

@fiseni
Copy link
Collaborator

fiseni commented Oct 24, 2020

@ardalis What's your thought on this?

If we decide to utilize expression reconstructions, then we can achieve a lot, and add quite many features (btw, we can solve #38 too).
But, on the other hand, it's kind of a rabbit hole if we go on that path. There are a lot of edge cases. We should be more carefully following .NET changes, and we should include rigorous tests for everything.

@ardalis
Copy link
Owner

ardalis commented Nov 2, 2020

If calling code is having to AND and OR different specifications together, that's essentially query logic the caller is responsible for. I'd put that logic into a new specification so the caller doesn't need to do anything. In that new specification, it can AND and OR however it wants in its Query construction. To me giving callers the ability to combine specifications just asks for query logic to leak up the call stack.

@ardalis
Copy link
Owner

ardalis commented Dec 4, 2020

Closing as I think this can/should be done in individual specifications to keep logic from leaking throughout the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants