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

make Int64 aka JavaLong conform to JavaParameterConvertible #11

Merged
merged 2 commits into from
May 25, 2018

Conversation

michaelknoch
Copy link
Collaborator

This adds the ability to pass Int64/JavaLongs between worlds.
I'm not quite sure if the naming of the functions is correct. Do we want to getInt64Fields or do we want to getLongFields?

// Int64 aka JavaFloat

extension Int64: JavaParameterConvertible, JavaInitializableFromMethod, JavaInitializableFromField {
public static let asJNIParameterString = "L"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"L" is the type signature for a class. "J" should be the right string here according to https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html#wp276

public static func fromField(_ fieldID: JavaFieldID, on javaObject: JavaObject) throws -> Int64 {
return try jni.GetInt64Field(of: javaObject, id: fieldID)
}
}
Copy link
Collaborator

@rikner rikner May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use "Long" in function Names instead of "Int64" since the original JNI API also uses "Long" (GetLongField()etc.) and I think we should stick to it.
Though I see that from a Swift standpoint this might be weird.
Not sure what @ephemer thinks about that.

@michaelknoch michaelknoch force-pushed the allow-passing-longs branch from 1d9db4f to 327e1ff Compare May 25, 2018 10:02
Copy link
Collaborator

@rikner rikner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would wait for @ephemer s opinion about the naming

@ephemer
Copy link
Member

ephemer commented May 25, 2018

I agree with @rikner in that we should call it GetLongXXX: not necessarily because it's called that in JNI but because it's called that in Java. Our method signature would then be e.g. GetLongField(of javaObject: JavaObject, id fieldID: JavaFieldID) @michaelknoch @rikner

We're getting what we defined in Java as a long, and it just happens to return an Int64 in Swift


// Int64 aka JavaLong

extension Int64: JavaParameterConvertible, JavaInitializableFromMethod, JavaInitializableFromField {
Copy link
Member

@ephemer ephemer May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should define this as an extension on JavaLong, and return / cast values to JavaLong wherever relevant - it's the same type, but it makes the API's intent clearer

public static let asJNIParameterString = "J"

public func toJavaParameter() -> JavaParameter {
return JavaParameter(long: Int64(self))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to recast a JavaLong to a JavaLong :)

@ephemer
Copy link
Member

ephemer commented May 25, 2018

The commit message about JavaFloat is pretty confusing - a JavaFloat is just a Float, and isn't related to JavaLong at all

@michaelknoch michaelknoch force-pushed the allow-passing-longs branch from a1b4639 to 2497791 Compare May 25, 2018 11:26
@ephemer ephemer merged commit 25a5140 into SwiftAndroid:devel May 25, 2018
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.

3 participants