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

naming: delete vs release #707

Closed
dcharkes opened this issue Nov 9, 2022 · 5 comments · Fixed by dart-archive/jnigen#379
Closed

naming: delete vs release #707

dcharkes opened this issue Nov 9, 2022 · 5 comments · Fixed by dart-archive/jnigen#379

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Nov 9, 2022

We have delete, deleteIn and in dart-archive/jnigen#130 deleteOriginal.

"delete" refers to deleting the handle that holds on to the Java object (DeleteGlobalRef).

However, the method is on an object representing a Java object. And we're not actually deleting any Java objects. Instead, we're letting go of the Java object.

Someone using our API does not know about handles, they just know that we are "referring" to a Java object (JReference).

So, we should consider renaming "delete" to something that signals we're no longer holding on, no longer referring to the Java object. For example "release". You release the object, the opposite of holding on. (Or maybe the opposite of holding on would be "let go" but that is two words.) Or another option is we "forget" about the Java object, but that seems like we're in control of what we're doing. So, "release"?

Some prior discussion: #766

@mahesh-hegde I want to move more and more away from the JNI implementation details. 🙃
@HosseinYousefi what are your thoughts about this?

@mahesh-hegde
Copy link
Contributor

I want to move more and more away from the JNI implementation details. 🙃

Not to discourage you

https://www.joelonsoftware.com/2002/11/11/the-law-of-leaky-abstractions/

😅

@HosseinYousefi
Copy link
Member

what are your thoughts about this?

I agree that delete is not be the best name for it.

@mahesh-hegde
Copy link
Contributor

What's the status of this?

One issue I see is release() may be mistaken for all instances of that object, possibly obtained through different calls, being released. But they are all different JNI references with independent lifecycle.

@HosseinYousefi
Copy link
Member

One issue I see is release() may be mistaken for all instances of that object,

Same can be said about delete. We used to have [obj release] in Objective-C which meant let's just reduce the reference counter by one.

@HosseinYousefi
Copy link
Member

Let's decide on this and #700 before 0.6.0 release.

Mandatory @lrhn tag.

Currently we have javaObject.delete() meaning that we want to let go of this one reference to the object, not to completely get rid of it. I personally feel like release captures this concept better. Any other ideas?

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

Successfully merging a pull request may close this issue.

3 participants