mbuhot/eskotlin

Prefer extensions on existing types to new types storing only configuration

Closed this issue · 5 comments

Currently for constructing queries, you're using classes like RangeData for storing parameters. Then later you apply those parameters to the framework RangeQueryBuilder.

I wonder why you don't configure the framework builder directly:

inline fun range(name: String, block: RangeQueryBuilder.() -> Unit): RangeQueryBuilder =
        RangeQueryBuilder(name).apply(block)

(Or as an extension function inRange on String, similar to #27.)

That way we would leverage existing APIs instead of creating additional overhead.

To make configuring more dogmatic in Kotlin we should would rather use extension vars on the framework types like this:

var RangeQueryBuilder.includeLower: Boolean
    get() = includeLower()
    set(value) {
        includeLower(value)
    }

DSL-like queries would then still be possible:

range("age") {
    from = 10
    to = 20
    includeLower = false
}

(Or "age" inRange { ... } as an extension function.)

I would like to help you there, if you need any help 😉

You may note, that I am currently working on a similar project currently.
As I'm implementing a DSL bottom up, starting at the client, there won't be any conflicts soon.
But if you agree, I would like to join efforts and we could merge the two libraries!

That's a nice suggestion. Though as your example shows, the required constructor parameters must be provided separately to the extension vars, so having the separate data classes allows us to mimic the JSON Query DSL more closely in this case.

I would like to join efforts and we could merge the two libraries!

EsKotlin is practically in maintenance mode. It get occasional contributions but it’s pretty stable.

I appreciate the sentiment, but it might be more practical to fork/copy EsKotlin into your new project and hack on it a bit. Try out some of the ideas to make it more like a modern idiomatic Kotlin code base.

I find it a great idea to fork EsKotlin. I'm not that expreienced with git, but I'll try to merge your extensions to my repo and I'll see If that works out. Because if I merge your module, at least the commit history should not be lost.
See janheinrichmerker/elasticsearch-ktx#1 for reference.