Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kotlin class support #47

Closed
marcoRS opened this issue Apr 13, 2015 · 13 comments
Closed

Kotlin class support #47

marcoRS opened this issue Apr 13, 2015 · 13 comments

Comments

@marcoRS
Copy link

@marcoRS marcoRS commented Apr 13, 2015

Is there planned support for classes written in Kotlin? Currently members in a Kotlin class annotated with "Icicle" do not get saved.

@frankiesardo
Copy link
Owner

@frankiesardo frankiesardo commented Apr 13, 2015

Nothing planned for now, I don't have much experience with Kotlin. That could change in a couple of months tho, so I'll keep this issue open.

I know Jake ported his ButterKnife to Kotlin, so it shouldn't be too hard. Pull requests or more info about what's required to make it work with Kotlin greatly appreciated.

@dalewking
Copy link

@dalewking dalewking commented May 30, 2015

Kotterknife, which is Jake's version of Butterknife view injection is not really a port and does not use annotation processing at all. He instead used Kotlin's property delegation support, which would not work well for Icepick.

The bottom line is that Kotlin does not support raw field access from one class to another. It has properties not fields from which it generates a private field and get/set methods.

What Icepick would need to do is add support for properties (using get/set methods) in addition to raw field access. That should be supported in 2 ways:

  • Allow Icicle to be applied to methods where it would be applied to get and set methods where Icepick processor would have to verify that it has both a get/set method for the property. There is all the stuff for reflecting on properties in java.beans.Introspector which you would use.
  • If you find a field on a class annotated with Icicle that is private first check to see if the class defines a property of the same name (using the Introspector)

The first case above lets you do this:

var savedValue : String? = null
    @Icicle set
    @Icicle get

where set and get could be customized such that there actually is no generated field. Future versions of Kotlin will allow this to be more concise.

The second case is the more basic:

@Icicle var savedValue : String? = null

which applies the Icicle annotation to the generated private field but generates get and set methods.

Note that adding property support would actually be useful in Java as well and would allow you to have get/seet for things you want to save that actually are computed without actually having backing fields.

@dalewking
Copy link

@dalewking dalewking commented May 30, 2015

Here are the relevant parts of Kotlin documentation:

http://kotlinlang.org/docs/reference/properties.html

There is also this: http://kotlinlang.org/docs/reference/java-interop.html, but the only relevant line is:

Property getters are turned into get-methods, and setters – into set-methods.

@rciurkot
Copy link

@rciurkot rciurkot commented Nov 10, 2015

Simply using setters and getters when field is not accessible would make it work. Please refer to this: sockeqwe/fragmentargs#21
However I'd like to continue to use Icepick as I find it more useful and nicer :)

@dalewking
Copy link

@dalewking dalewking commented Nov 11, 2015

The addition of the @JvmField annotation to Kotlin makes this request just a nice to have.

@rciurkot
Copy link

@rciurkot rciurkot commented Nov 12, 2015

@dalewking Yes, handling only fields is not a deal-breaker when @State is used in conjuction with @JvmField (lateinit does a similar thing).
However forcing having non-private fields (whether in Java or Kotlin) is not a good programming practice. There's a reason why all Java books tell you to write private fields with public setters/getters.

Nonetheless, as you say, it'd be nice to have setters and getters being handled properly by Icepick.
P.S. I'd love to make a PR with this but clojure is not my cup of tea :(

@frankiesardo
Copy link
Owner

@frankiesardo frankiesardo commented Nov 12, 2015

Thanks all for the useful information shared in this discussion.

If there is an effective workaround at the moment to get Icepick working with Kotlin I'd be inclined not to add complexity and change the codebase. Kotlin is a fast evolving language and adding explicit support for it makes me chase a moving target.

Happy to reconsider it in the future if the conditions change but closing it for now.

@clemp6r
Copy link

@clemp6r clemp6r commented Nov 12, 2015

FYI Kotlin has reached its 1.0.0 and is not a moving target anymore.

Le jeu. 12 nov. 2015 21:58, Frankie Sardo notifications@github.com a
écrit :

Closed #47 #47.


Reply to this email directly or view it on GitHub
#47 (comment).

@rciurkot
Copy link

@rciurkot rciurkot commented Nov 13, 2015

Also it's not about making a Kotlin-specific change but safe-and-efficient-Java-coding change. As I said before: "There's a reason why all Java books tell you to use private fields with non-private setters/getters."
(And as @clemp6r said - Kotlin is stable now)

@tinsukE
Copy link

@tinsukE tinsukE commented Aug 19, 2016

If you're not a fan of exposing your variables and using @JvmField, I've started a simple lib that should work as Icepick: https://github.com/tinsukE/icekick
Contributions are welcome!

@ZakTaccardi
Copy link

@ZakTaccardi ZakTaccardi commented Dec 1, 2016

what is the workaround here in Kotlin?

I tried the following, without success.

@JvmField @State var activeDrawerOption: DrawerOption = DrawerOption.SALES

EDIT: this works, had a build issue preventing annotation processing

@vRallev
Copy link

@vRallev vRallev commented Jan 26, 2017

And here's another alternative. Same API like Icepick, but works well with properties: https://github.com/evernote/android-state

@joseph-acc
Copy link

@joseph-acc joseph-acc commented Jan 6, 2020

@ZakTaccardi do you have an example of it working?
I tried it and didn't have success.
Put the variable within a companion object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.