mbuhot / eskotlin

Elasticsearch Query DSL for Kotlin
MIT License
136 stars 28 forks source link

Consider use invoke operator overloading instead of extension methods named 'to'? #16

Closed vincentlauvlwj closed 6 years ago

vincentlauvlwj commented 6 years ago

For example, when I'am constructing a range query, I write codes like this:

val query = range {
    "age" to {
        gte = 10
        lte = 20
        boost = 2.0f
    }
}

This 'to' method is declared in RangeBlock class like:

infix fun String.to(init: RangeData.() -> Unit): RangeData {
     return RangeData(name = this).apply(init)
}

But I think it is better to use invoke operator overloading here, just replace the 'to' method with this:

operator inline fun String.invoke(init: RangeData.() -> Unit): RangeData {
    return RangeData(name = this).apply(init)
}

Then we can write a query like this:

val query = range {
    "age" {
        gte = 10
        lte = 20
        boost = 2.0f
    }
}

Now, we don't need to use the ugly key word anymore, the query looks clean.

Please consider this, or may I implement this myself and send a pull request to you?

mbuhot commented 6 years ago

@vincentlauvlwj That syntax looks very nice! Please send a PR.

For compatibility, I'd prefer not to remove the to syntax completely, so can we make to just call invoke? Perhaps we can put a @Deprecated annotation on to methods that are no longer needed.

We'll need to keep the to syntax for the cases where the right side is just a string, eg: Term

vincentlauvlwj commented 6 years ago

@mbuhot I just sent a PR, please check and merge it.