Skip to content

Commit

Permalink
implement Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
rikner committed Aug 25, 2017
1 parent 99c444e commit 2babba9
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 51 deletions.
18 changes: 7 additions & 11 deletions Sources/JNI/JNIFields.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,19 @@

import CJNI

struct FieldIDNotFound: Error {}

public extension JNI {
public func GetStaticField<T: JavaParameterConvertible>(_ fieldName: String, on javaClass: JavaClass) throws -> T {
let env = self._env
guard let fieldID = env.pointee.pointee.GetStaticFieldID(env, javaClass, fieldName, T.asJNIParameterString) else {
throw FieldIDNotFound()
}
return try T.fromStaticField(of: javaClass, id: fieldID)
let fieldID = env.pointee.pointee.GetStaticFieldID(env, javaClass, fieldName, T.asJNIParameterString)
try checkAndThrowOnJNIError()
return try T.fromStaticField(of: javaClass, id: fieldID!)
}

public func GetField<T: JavaParameterConvertible>(_ fieldName: String, from javaObject: JavaObject) throws -> T {
let env = self._env
guard let javaClass = GetObjectClass(obj: javaObject),
let fieldID = env.pointee.pointee.GetFieldID(env, javaClass, fieldName, T.asJNIParameterString) else {
throw FieldIDNotFound()
}
return try T.fromField(of: javaObject, id: fieldID)
let javaClass = try GetObjectClass(obj: javaObject)
let fieldID = env.pointee.pointee.GetFieldID(env, javaClass, fieldName, T.asJNIParameterString)
try checkAndThrowOnJNIError()
return try T.fromField(of: javaObject, id: fieldID!)
}
}
22 changes: 10 additions & 12 deletions Sources/JNI/JNIObjects.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import CJNI

struct CreateNewObjectForConstructorError: Error {}
struct ClassNotFound: Error {}
struct ConstructorError: Error {}

/// Designed to simplify calling a constructor and methods on a JavaClass
Expand All @@ -13,18 +12,16 @@ open class JNIObject {
public init(_ className: String, arguments: [JavaParameterConvertible] = []) throws {
let className = className.replacingFullstopsWithSlashes()

guard
let javaClassLocalRef = jni.FindClass(name: className),
let javaClass = jni.NewGlobalRef(javaClassLocalRef)
else {
assertionFailure("Couldn't find class named \(className)")
throw ClassNotFound()
}

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

I think FindClass itself should throw, rather than checking afterwards

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

Otherwise it'll probably crash before it gets to throw

let javaClassLocalRef = jni.FindClass(name: className)
try checkAndThrowOnJNIError()

self.javaClass = javaClass
let javaClass = jni.NewGlobalRef(javaClassLocalRef!)
try checkAndThrowOnJNIError()

self.javaClass = javaClass!

guard
let instanceLocalRef = try? jni.callConstructor(on: javaClass, arguments: arguments),
let instanceLocalRef = try? jni.callConstructor(on: self.javaClass, arguments: arguments),
let instance = jni.NewGlobalRef(instanceLocalRef)
else {
assertionFailure("Error during call to constructor of \(className)")
Expand Down Expand Up @@ -72,10 +69,11 @@ public extension JNI {
return newObject
}

public func GetObjectClass(obj: JavaObject) -> JavaClass? {
public func GetObjectClass(obj: JavaObject) throws -> JavaClass {
let env = self._env
let result = env.pointee.pointee.GetObjectClass(env, obj)
return (result != nil) ? result : .none
try checkAndThrowOnJNIError()
return result!
}

}
13 changes: 5 additions & 8 deletions Sources/JNI/JavaParameterConvertible.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ extension Bool: JavaParameterConvertible {
}

public static func fromField(of javaObject: JavaObject, id: jfieldID) throws -> Bool {
return jni.GetBooleanField(of: javaObject, id: id) == 1
return try jni.GetBooleanField(of: javaObject, id: id) == JNI_TRUE
}
}

Expand All @@ -65,7 +65,7 @@ extension Int: JavaParameterConvertible {
}

public static func fromField(of javaObject: JavaObject, id: jfieldID) throws -> Int {
return Int(jni.GetIntField(of: javaObject, id: id))
return try Int(jni.GetIntField(of: javaObject, id: id))
}
}

Expand All @@ -77,8 +77,7 @@ extension Double: JavaParameterConvertible {
}

public static func fromStaticField(of javaClass: JavaClass, id: jfieldID) throws -> Double {
let result = try jni.GetStaticDoubleField(of: javaClass, id: id)
return Double(result)
return try Double(jni.GetStaticDoubleField(of: javaClass, id: id))
}

public static func fromMethod(calling methodID: JavaMethodID, on object: JavaObject, args: [JavaParameter]) throws -> Double {
Expand All @@ -90,7 +89,7 @@ extension Double: JavaParameterConvertible {
}

public static func fromField(of javaObject: JavaObject, id: jfieldID) throws -> Double {
return Double(jni.GetDoubleField(of: javaObject, id: id))
return try jni.GetDoubleField(of: javaObject, id: id)
}
}

Expand Down Expand Up @@ -119,9 +118,7 @@ extension String: JavaParameterConvertible {
}

public static func fromField(of javaObject: JavaObject, id: jfieldID) throws -> String {
guard let javaStringObject = jni.GetObjectField(of: javaObject, id: id) else {
throw FieldIDNotFound()
}
let javaStringObject = try jni.GetObjectField(of: javaObject, id: id)
return jni.GetString(from: javaStringObject)
}
}
44 changes: 24 additions & 20 deletions Sources/JNI/SwiftJNI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,10 @@ extension JNI {
let objectClass = _env.pointee.pointee.GetObjectClass(_env, object)
try checkAndThrowOnJNIError()

guard let result = _env.pointee.pointee.GetMethodID(_env, objectClass!, methodName, methodSignature) else {
throw InvalidMethodID()
}

let result = _env.pointee.pointee.GetMethodID(_env, objectClass!, methodName, methodSignature)
try checkAndThrowOnJNIError()
return result

return result!
}

public func GetStaticMethodID(for javaClass: JavaClass, methodName: String, methodSignature: String) throws -> JavaMethodID {
Expand Down Expand Up @@ -164,24 +162,32 @@ extension JNI {

// MARK: Fields

public func GetBooleanField(of javaObject: JavaObject, id: jfieldID) -> JavaBoolean {
public func GetBooleanField(of javaObject: JavaObject, id: jfieldID) throws -> JavaBoolean {
let _env = self._env
return _env.pointee.pointee.GetBooleanField(_env, javaObject, id)
let result = _env.pointee.pointee.GetBooleanField(_env, javaObject, id)

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

I guess this should break currently if the field is not found, we probably need to fix the annotation in jni.h to specify that it returns a nullable javaboolean

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

Actually: C has no concept of optionals. I actually have no idea what happens here on failure. I think it's actually fine as is, sorry!

try checkAndThrowOnJNIError()
return result
}

public func GetIntField(of javaObject: JavaObject, id: jfieldID) -> JavaInt {
public func GetIntField(of javaObject: JavaObject, id: jfieldID) throws -> JavaInt {
let _env = self._env
return _env.pointee.pointee.GetIntField(_env, javaObject, id)
let result = _env.pointee.pointee.GetIntField(_env, javaObject, id)

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

Same for these. But it's hard to predict the behaviour without tests

try checkAndThrowOnJNIError()
return result
}

public func GetDoubleField(of javaObject: JavaObject, id: jfieldID) -> JavaDouble {
public func GetDoubleField(of javaObject: JavaObject, id: jfieldID) throws -> JavaDouble {
let _env = self._env
return _env.pointee.pointee.GetDoubleField(_env, javaObject, id)
let result = _env.pointee.pointee.GetDoubleField(_env, javaObject, id)
try checkAndThrowOnJNIError()
return result
}

public func GetObjectField(of javaObject: JavaObject, id: jfieldID) -> JavaObject? {
public func GetObjectField(of javaObject: JavaObject, id: jfieldID) throws -> JavaObject {
let _env = self._env
return _env.pointee.pointee.GetObjectField(_env, javaObject, id)
let result = _env.pointee.pointee.GetObjectField(_env, javaObject, id)
try checkAndThrowOnJNIError()
return result!

This comment has been minimized.

Copy link
@ephemer

ephemer Aug 25, 2017

Member

This is still correct though I think, because references are optional / nullable

}

// MARK: Static Fields
Expand Down Expand Up @@ -315,9 +321,9 @@ extension JNI {
if (index >= count) {
throw JNIError()
}
let jObj = _env.pointee.pointee.GetObjectArrayElement(_env, array, jsize(index))!
let jObj = _env.pointee.pointee.GetObjectArrayElement(_env, array, jsize(index))
try checkAndThrowOnJNIError()
return jObj
return jObj!
}
}

Expand Down Expand Up @@ -381,7 +387,7 @@ public struct JavaCallback {
- parameters: Any number of JavaParameters (*must have the same number and type as the* `methodSignature` *you're trying to call*)
- Throws: `JavaCallback.Error`
*/
public init (_ globalJobj: JavaObject, methodName: String, methodSignature: String) {
public init (_ globalJobj: JavaObject, methodName: String, methodSignature: String) throws {
// At the moment we can only call Void methods, fail if user tries to return something else
guard let returnType = methodSignature.characters.last, returnType == "V"/*oid*/ else {
// LOG JavaMethodCallError.IncorrectMethodSignature
Expand All @@ -393,10 +399,8 @@ public struct JavaCallback {
// Doesn't work with more complex object types, arrays etc. we should determine the signature based on parameters.

// TODO: Check methodSignature here and determine expectedParameterCount

guard
let javaClass = jni.GetObjectClass(obj: globalJobj),
let methodID = try? jni.GetMethodID(for: javaClass, methodName: methodName, methodSignature: methodSignature)
let javaClass = try jni.GetObjectClass(obj: globalJobj)
guard let methodID = try? jni.GetMethodID(for: javaClass, methodName: methodName, methodSignature: methodSignature)
else {
// XXX: We should throw here and keep throwing til it gets back to Java
fatalError("Failed to make JavaCallback")
Expand Down

0 comments on commit 2babba9

Please sign in to comment.