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

added more enum handling. Now enum of type works, and enum with set v… #93

Closed
wants to merge 1 commit into from

Conversation

torlangballe
Copy link
Collaborator

Adding this early before adding tests etc to get feeback; Is this a good idea or are there better ways to do it. Existing code now uses some "sealed class" that might be a better way?

Being able to init a simple enum from a raw value is very useful though, so some way of doing that is pretty important.

…alues will output a helper class method in enum to create from raw values
@angelolloqui
Copy link
Owner

angelolloqui commented Feb 18, 2019

Just by looking at the code it gets pretty hard to follow the reasoning for all your cases, but the usage of sealed classes is definitely the way to go to convert complex Swift enums.

What called my attention on your changes is the usage of ZEnumCompanion. We should avoid custom types like this to keep the tool neutral.

Can you give some examples of the desired conversion Swift->Kotlin that you are trying to get?

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

I think I am still not understanding your usecase. For example, if you want to initialize the enum from a rawValue, wouldn't this fit your needs?

enum class Num(val rawValue: Int) {
    Five(5),
    Six(6),
    Seven(7);

    companion object {
        fun fromRawValue(rawValue: Int): Num? =
                Num.values().firstOrNull { it.rawValue == rawValue }
    }
}

With it, then you could use the enum like:

val myEnum = Num.fromRawValue(5)

Is your usecase different?

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

I think giving this fromRawValue (or similar) makes sense indeed, since you can do the same with Swift out of the box. The only point I dislike is that from Swift you would do:

let myEnum = Num(rawValue: 5)

Which is not the same than the suggested Kotlin counterpart, and will therefore not translate properly... Maybe (I am not sure), we could try to detect that you are instantiating an enum in swift to convert it to a call to fromRawValue, but I suspect that this would not be possible since we do not have data types for the expression.

@angelolloqui
Copy link
Owner

BTW, in your example with the ZEnumCompanion, how would you use it from Kotlin when you want to instantiate Num?

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

All right, that is indeed what I suspected. I do not see any other way with the information about types we have...

This piece of code, is it part of a plugin? I am doubting if we should provide the whole feature of fromRawValue (adding the method and converting the calls) as part of the main core or as a post processor plugin (so people could disable)

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

Link to the issue tracking: #94

@angelolloqui
Copy link
Owner

Some things would be very usefull for users of the system to just be able to turn on and off, a plugin just moves that process outside main converter.

Yes, my idea behind the plugins was to provide hooks to add extra converters that not everyone will want to use and that are not part of the language itself. For example, a converter between RxSwift and RxKotlin or a converter between UIKit and Android widgets would fit on this category.

But this one is on the limit, since the enum(rawValue:) is a Swift feature, from the language, but it does not exists in Kotlin itself. From Swift perspective it should be on the core, but from Kotlin perspective is an artificial companion object we are creating that might not be even wanted (for example if you have other initializers). My feeling so far is that it should be part of the core, since it is a Swift language feature, but I will think about it. What do you think?

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

Take a look to my last comment on #94. If that works, it gets the best from both worlds.

@torlangballe
Copy link
Collaborator Author

torlangballe commented Feb 18, 2019 via email

@angelolloqui
Copy link
Owner

Is this one still valid? I think with #97 this is no longer needed right?

@torlangballe
Copy link
Collaborator Author

Hi, yes, #97 was the new simpler version I did. Great, I'll have a look at you changes!

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

Successfully merging this pull request may close these issues.

2 participants