Tornadofx: An ItemViewModel doesn't commit the changes of a nullable field into the item

Created on 6 Oct 2017  路  7Comments  路  Source: edvin/tornadofx

That might be just a mistake of mine, yet still...

When I create an ItemViewModel like that:

class Entity {
    var field: String? = null
}

class EntityModel : ItemViewModel<Entity> {
    val field = bind(Entity::field)

    constructor(item: Entity) {
        this.item = item
    }
}

And then try to commit() the changes, for example:

val entityModel = EntityModel(item = Entity())
entityModel.field.value = "some value"
entityModel.commit()
assertTrue(entityModel.item.field == null)

I end up with a null.

While if I change the line var field: String? = null to var field: String = "" the commit() works perfectly fine.

If that behavior is expected, then I believe an exception on the binding val field = bind(Entity::field) would be quite useful.

I have created a repository with a working example: an app example and the test case from above.

Most helpful comment

Merged :) Good job!

All 7 comments

I believe this was reported and fixed just a few days ago. Can you try with the latest snapshot at oss.sonatype.org?

Sure.

But because I was not able to find any official instructions for gradle, I'm not certain that I've done everything correctly. My commit to the build.gradle.

The problem does persist with that configuration.

Your Gradle config is correct. The fix was actually rolled back as it introduced more problems than it solved. If possible, use observable properties in your Entity for now, that's the use case the ViewModel was created for handling, although sopport for Pojo/Poko has been added to some degree.

Due to work with Java 9 conversion this has a pretty low priority now, at least for me, but we can leave the issue open so perhaps others can contribute. Sorry about that :)

@chinzen, @edvin

I believe I nailed it ;) long strony short, in that case bind invocation falls to KProperty instead of KMutableProperty, so it was just treated lik a read-only field, but without any obvious errors or compilation warnings.

Pull request #493

Oh wow, that's fantastic news! I will look this over and merge soon!

@edvin sure thing :)

The fastest way to find what's going on is to comment out that new bind variant from ViewModel.kt and then running poko_commit() test from ViewModelTest.kt. Note which bind variants are called from PersonPokoModel in the same file. Anyway, test should fail.

Then uncomment it and run the test again. This time it should succeed. Again, take note which bind variants are called this time.

Of course I could have missed something - let me know if I got it wrong, a review is always welcome :)

Merged :) Good job!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vlipovetskii picture vlipovetskii  路  6Comments

MairwunNx picture MairwunNx  路  3Comments

iwh718 picture iwh718  路  3Comments

leder11011 picture leder11011  路  5Comments

orochies picture orochies  路  4Comments