edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.68k stars 272 forks source link

Call class property which name start whit "is" #667

Open iyfedorov opened 6 years ago

iyfedorov commented 6 years ago

While call class field as a property which start with "is" throwing java.lang.NoSuchMethodException Example:

class ModelClass {
    var isPercentagePayment: String? = null }
    val model: Any = ModelClass() // class instance with field "isPercentagePayment"
    val fieldName = "isPercentagePayment" // field name
    val text = model.observable<String>(fieldName)?.value ?: ""
}

this throw java.lang.NoSuchMethodException: ModelClass.getIsPercentagePayment()

torndofx version : 2.7.15 kotlin version : 1.2.30

edvin commented 6 years ago

Your example is quite contrived, and even impossible to run, as an instance of ModelClass would create a new instance of ModelClass in it's constructur, which again would create a new ModelClass and so on. Please supply an actual/meaningful code example and mention what it is you're trying to achieve :)

tieskedh commented 6 years ago

@edvin

A getter starts with get with normal types and is with booleans. Because this field starts with is, Kotlin chooses to implement the getter as if it was a boolean. This means the getter is isPercentagePayment() and the setter is setPercentagePayment()

our code does only checks if a property starts with get: https://github.com/edvin/tornadofx/blob/master/src/main/java/tornadofx/Properties.kt#L126

The question is, do we want to fix this, or do we stick with the conventions?

edvin commented 6 years ago

I think it would be good to fix this, it will only cause confusion IMO. You up for it? :)

tieskedh commented 6 years ago

@edvin can we use different parameters? I think Getter and Setter work much better as they already contain the property. The drawback is that java doesn't have them.

I currently have the following code:

fun <S : Any, T : Any> S.observable(
        getter: KFunction<T>? = null,
        setter: KFunction2<S, T, Unit>? = null,
        propertyName: String? = null,
        @Suppress("UNUSED_PARAMETER") propertyType: KClass<T>? = null
): ObjectProperty<T> {
    if (getter == null && propertyName == null) throw AssertionError("Either getter or propertyName must be provided")

    val builder = JavaBeanObjectPropertyBuilder.create()
    if (propertyName != null){
        builder.name(propertyName)
    } else {
        val getName = getter!!.name
        if (getName.startsWith("get")){
            builder.name(getName.substring(3).decapitalize())
        } else {
            assert(getName.startsWith("is"))
            val props = this::class.memberProperties.map { it.name }
            if(getName in props) {
                builder.name(getName)
            } else {
                val possiblePropsName = getName.substring(2).decapitalize()
                if (possiblePropsName in props) builder.name(possiblePropsName)
            }
        }
    }
    setter?.also { builder.setter(it.javaMethod) }
    getter?.also { builder.getter(it.javaMethod) }
    return builder.build() as ObjectProperty<T>
}

As you can see, I take every property to see if it matches the name. I'm not sure if this does work with java, otherwise we have to change it, if we still support java

edvin commented 6 years ago

We have to make sure to still support Java objects, as they can be pulled in through dependencies or otherwise be unchangeable for the developer.