Java: rename the delete function to _internalDelete and make private

[tasn]

Hey,

At the moment, Java classes are created with a special delete function that is in charge of clearing the Rust object, e.g.:

    public synchronized void delete() {
        if (mNativeObj != 0) {
            do_delete(mNativeObj);
            mNativeObj = 0;
       }
    }

Delete is quite a useful name to have (I use it in my API) and this function being called this way is confusing, but also needlessly limits the API. It would be much better if the function was private (so it's not autocompleted by editors) and called something else so the delete name can be used in API.

Thanks!

[Dushistov]

Not sure about fixed name, may be I will add option to regulate naming scheme.
Should be done after #303

[tasn]

Would you accept a PR where I rename delete and make it private? I can even call it _delete if you don't like the other name.
I need it for my API, and I would really rather not depend on my own flapigen fork.

Thanks!

[tasn]

I have what seems to be a working patch, though I think _delete is a terrible name. internal_delete or rust_delete or instance_delete should be better. What do you think?

[Dushistov]

Would you accept a PR where I rename delete and make it private?

Such change can broke code of other users.
If user want to free memory right after it not used anymore:

var big = new BigRustObject();
//do something
big.delete();

with such change their code would be broken,
and without any viable option to fix it.
May be the generated classes should implement Closebale.
That's why I think about fixing #303 before this.

I would really rather not depend on my own flapigen fork

Actually you don't need fork, you can use new extension method:
https://docs.rs/flapigen/0.6.0-pre5/flapigen/struct.Generator.html#method.register_class_attribute_callback

it takes generated java code as &mut Vec<u8>, so you can modify it in any way you want.
replacing b"public synchronized void delete()" bytes trivial.

[tasn]

flapigen 0.6.0 is not yet released, now is exactly the time to change it (rather than later), no?
Yeah, maybe close() would be better, and have it implement closeable. Though why this special handling? Normal java objects don't have a way to immediately ask to free all of their data...

As for your suggestion, I don't think it will work, as I believe it breaks even before that (I'm getting rust compilation errors, look at my patch, the destructor is registered as a function too).

[Dushistov]

As for your suggestion, I don't think it will work

You get full java code in this callback, how it is possible that this will not work ¯_(ツ)_/¯ ,
obviously you need also replace finilize function to call new internalDeleteOrSomeOtherName.

flapigen 0.6.0 is not yet released, now is exactly the time to change it (rather than later), no?

Changing is possible, but not breaking without fix changing. Renaming is one thing, users can just change method name in their code,
but "public -> private" change leaves them without any possibility to fix.

Normal java objects don't have a way to immediately ask to free

Yes, but if user uses native code from JVM the probability that he/she uses something not trivial increases.
Like bluetooth socket, solving some math problems with gigabytes of memory and so on and so on.
All these cases requires manual control for allocated resources.

[tasn]

As for your suggestion, I don't think it will work

You get full java code in this callback, how it is possible that this will not work ¯_(ツ)_/¯ ,
obviously you need also replace finilize function to call new internalDeleteOrSomeOtherName.

As I said, the error I'm getting is before the Java part, it's in Rust. The Rust compiler is complaining... I'm not sure, maybe it will, work, but I don't think it will, and I've settled on running my own fork of flapigen for now.

flapigen 0.6.0 is not yet released, now is exactly the time to change it (rather than later), no?

Changing is possible, but not breaking without fix changing. Renaming is one thing, users can just change method name in their code,
but "public -> private" change leaves them without any possibility to fix.

Gotcha.

Normal java objects don't have a way to immediately ask to free

Yes, but if user uses native code from JVM the probability that he/she uses something not trivial increases.
Like bluetooth socket, solving some math problems with gigabytes of memory and so on and so on.
All these cases requires manual control for allocated resources.

I don't see how that's different to any other Java code that could hold a lot of resources. If you want manual control, expose some manual control in the API and free the resources. I think this is very much a corner case rather than the main one, and anyway, by not doing anything you enable both, while what's there now is unnecessarily strict for many use cases.