and/or chaining ambiguity

[MoridinBG]

I am not entirely sure I follow the logic behind the AND/OR chaining of filters, when building queries. Using the example

Query<User>().filter("name", .Equals, "John").or { query in
    query.filter("phone", .NotEquals, "2234567890").and { query in
        query.filter("address", .Equals, "100 Apt Ln").filter("other", 1)
   }
}

I read this as name = ... OR (phone = ... AND (address = ... AND other = ...)) But the resulting sql statement is name = ... AND (phone != ... OR (address = ... AND other = ...))

I guess the underlying intention is that OR/AND is applied between the array of filters in the closure, with the result being AND-ed to the previous node in the filter chain. But the code reads as if it is OR between the previous node and the filters in the closure.

[tanner0101]

You're right, this could be a little confusing. What .or { query ... means is that the query variable inside the closure will separate all filters by an OR.

This is how Eloquent in Laravel works, so best to follow suit and not reinvent the wheel in my opinion.

[MoridinBG]

Don't argue with that, but perhaps the naming could be improved a bit.

[Evertt]

How about renaming and and or to both and either respectively? So then we get:

Query<User>().filter("name", .Equals, "John").either { query in
    query.filter("phone", .NotEquals, "2234567890").both { query in
        query.filter("address", .Equals, "100 Apt Ln").filter("other", 1)
   }
}

[Joannis]

Our MongoKitten's QueryBuilder might provide a solution here?

[Evertt]

I looked it up and I love it. Very clear and readable. I wonder how you would need to type "check if id is in [1,2,3]" in that syntax though. @Joannis, do you guys have a syntax for that too?

edit I guess we could just use the ~= operator for that.

let q: Query = "name" == "Evert" && "id" ~= [1,3,5]
// or use it with a range...
let q: Query = "name" == "Evert" && "id" ~= 1...10

[Joannis]

We don't.. but I think it should be possible for sure. It's actually a great idea! I love it :)

[Prince2k3]

I'm actually against operator overloading for the library for a few reasons. Call me a skeptic but I've notice a huge number of operator overloading abuse and would hope folks in the Vapor and Fluent community would think more about using them over an already clean syntax. Also It shouldn't be up to the library to use such things. Why? Well because not everyone is going to agree on the way to use operators. For example

let q: Query = "name" == "Evert" && "id" ~= [1,3,5]

I don't find this clear to read at all. Just from looking at it and not reading what you guys are saying it's confusing. My opinion is if a dev team that is using the library and THEY feel that it can be done with operator overloading then they should. Defining it for them is terrible.

[tanner0101]

Any other thoughts from @qutheory/fluent on how this should be implemented?

I have removed the current implementation (building side by side with SQL and Mongo now) and will propose a new one soon that we can vote on.

[Evertt]

@Prince2k3 why not both? Why not make both methods of composing filters available to the developers using Fluent and let them decide? I really like operators and I find them very easy to read and understand so I would heavily use them. You don't like operators in your projects, so you won't use them. And for people who work in a team, they will decide on which one they'll want to use in their projects.

[Prince2k3]

@Evertt Your miss understanding what I am saying. No one is preventing YOU from doing custom operators. But why force it in a library/framework? I am saying it shouldn't be in the framework. If you want it add it to your own project or even better create a github project that makes it an additive if you feel that your setup is the best and everyone should try to use it. One of the reason I like the Vapor/Fluent communities is that it keeps things simple and clean. It allows for folks like you to go an extend and do what you want to do. It's like Alamofire its simple and clean and doesn't do operator overloading it could if it wanted to but in it's evolution it doesn't require it.

[Evertt]

@Prince2k3 Yes I understand that you're saying it shouldn't be in the framework and if people want it then they should overload the operators in their own projects. Because, I hear you saying, you feel that putting this in the framework means that we're forcing it upon the users of our framework. Do I understand you correctly?

So to clarify what I'm saying: I say we should put these overloaded operators in the framework, because it's only additive. It doesn't force anyone to use them. They're just there if people want to use them. I don't understand how this is forcing anything on anyone... On the contrary, this offers people more ways of using our api and people can choose which one they like most.

[MoridinBG]

I am not super fan of framework specific operators either, as they differ from framework to framework and are not immediately obvious.

[tanner0101]

As long as we aren't operator only, I would like to support them. A large portion of the Swift community seems to really like them and want to use them.

My only concern would be ensuring we have proper unit testing in place since we would be doubling the amount of API for Filter

[timominous]

Allowing the users to implement filters using operators seems like a great idea here since the previous implementation was a little hard to understand. I agree that most frameworks have their own specific operators and differ from each other but we are not forcing the users to use them.

I think the matter of API dupplication is a bigger concern.