About upcoming version 3.0.0...

[unbiaseduser]

It looks like version 3.0.0 will force the users to use Kotlin coroutines to save images. While I'm normally all for progress and such, I really don't think that alienating your userbase is a good idea. IMO, Coroutines should only be an implementation detail to keep smooth transition from old to new versions.

I understand if you don't want to support the old API anymore, but seeing as even big companies' libraries still support not-coroutines users, this seems at least a little bit icky.

If you want, I can make PRs to solve all this once 3.0.0 drops.
Regards.

[wasky]

Old, non-suspending functions will be available but marked as deprecated.

[unbiaseduser]

Well yeah, I know. Should have worded it better (future versions instead of just 3.0.0). But they will be removed in the future, won't they? When that happens, that'll be the end of diversity and inclusion for not-coroutine users.

[unbiaseduser]

I know I'm just another person, but just hear me out: Exclusion of other people is not how you build healthy relationships in real life.

Like I said, I can work on this. It simply boils down to whether you're welcome of me. Please.

[burhanrashid52]

@unbiaseduser

Coroutines should only be an implementation detail to keep smooth transition from old to new versions.

Can you elaborate more here? Any example?

If you want, I can make PRs to solve all this once 3.0.0 drops.

Sure, if your PR solves the actual root problem, then I am happy to keep both version. cc @wasky

[unbiaseduser]

Implementation detail is basically how something works internally and not usually exposed to the outside world.
Example: The current version 2.x of this library uses Asynctask under the hood to save images. The fact that Asynctask is used is a textbook example of an implementation detail, and is (rightfully) not known by the user.

Why it matters

The current 2.x public API is fine imo. It may lack some fancy modern syntaxes, but it does the job well in ease of use. Hence, it accommodates everyone who wants to use it.
However, by coercing usage of a certain tool in future versions, that upside is basically lost. Now everyone who wants to use this library will have to learn that tool along with its nuances, not to mention that it's straight up impossible for people who cannot use that tool for whatever reason.
Does 2.x force the user to run a Asynctask everytime they want to save an image?
Does 2.x even expose the usage of Asynctask to the user? (Or, put it another way, does the user have to be actively aware that Asynctask is used under the hood?)
The answer is of course No! And that brings me to my point: smooth transition between versions. Imagine how surprised the user will feel when a supposed upgrade now forces them to use some tool, which there's a possibility that they simply cannot use, due to preference or other reasons.
A general use library should not assume anything about its users and which tools they use, whether they prefer tool X or tool Y, etc. no matter how great the coerced tool is. (Yes, I do agree that quasi-sequential Coroutines code is great).
A (rather extreme) example of a great library is Coil. Even though it literally stands for Coroutines, the basic unit of usage (just load some image and be done with it!) does not force Coroutines usage. That makes it strangely accessible to everyone interested, even if you can't make use of 100% functionality due to, well, Coroutines.

What can be done

Don't get me wrong, I think the work of (don't really wanna ping them) is awesome. It's just that in the process of revamping things, they left out a big hole in terms of accessibility. I simply seek to patch it up. From what I see, there are 2 options to do it:

• 1. Do not deprecate the 2.x public API. Since the functions there just delegate most of the work to the coroutines anyway, this solution has practically no downsides.
  • Easiest solution
  • Might not be "modern" enough for some people I guess?
• 2. Still deprecate the 2.x public API, and create a non-coroutine counterpart of the new 3.x API.
  • Requires a little more thought (but considering the effort already made, this should be a non-issue)
    That's all, I suppose. Just remember, technology should make things easier for everyone, and arbitrary decisions that do the opposite are an ethical breach.
    Regards.

[wasky]

Usage of new API would be as simple as the code below:

// In activity
lifecycleScope.launch {
    val result1 = photoEditor.saveAsFile(path)
}

// In fragment
viewLifecycleOwner.lifecycleScope.launch {
    val result1 = photoEditor.saveAsFile(path)
}

// If you don't want to use coroutines, a blocking version of the call.
// This can be used in deprecated `AsyncTask.doInBackground()`.
val result2 = runBlocking { photoEditor.saveAsFile(path) }

I believe that the usage of new API is simple enough that it can be used by developers with minimal knowledge about coroutines and I think we should not introduce new, non-coroutine API. New API means it has to be maintained. Less code = less maintenance = less potential bugs. I just don't see lots of benefits of the new API. Maybe I missing something?

Also I would prefer to keep old API marked as deprecated to encourage developers to use new API. We don't have to delete old API, just keep marked as deprecated. The reason for this is that old API behaves differently than new API. Old API will be using GlobalScope. That means that OnSaveListener.onSuccess() can be called after activity/fragment is destroyed. You have to remember to handle such case. If you use new API properly using lifecycleScope.launch { ... } you don't need to worry about activity lifecycle.

photoEditor.saveAsFile(path, object : PhotoEditor.OnSaveListener {

    override fun onSuccess(imagePath: String) {
        // This can be called after fragment/activity is destroyed,
        // imageView can be `null` for example.
        imageView.visibility = View.GONE
    }

    // ...

})

lifecycleScope.launch {
    photoEditor.saveAsFile(path)
    imageView.visibility = View.GONE // This won't be called after activity is destroyed
}

To help developers write code with less errors I think we should encourage developers to use new API by deprecating the old one.

And even if the old API will be deleted in the future, you can reintroduce it in your own app simply by adding the code below to your project:

PhotoEditorExtensions.kt

fun PhotoEditor.saveBitmapAsync(path: String, listener: SaveBitmapListener) {
    GlobalScope.launch(Dispatchers.Main) {
        when (val result = saveAsFile(path)) {
            SaveFileResult.Success -> listener.onSuccess()
            is SaveFileResult.Failure -> listener.onFailure(result.exception)
        }
    }
}

interface SaveBitmapListener {
    fun onSuccess()
    fun onFailure(e: IOException)
}

Can you provide any real-life scenario where coroutines cannot be used? It would be easier to address any specific issue you have with the new approach.

[unbiaseduser]

*Let's get this right out of the bat: My concerns are not specifically about me. I know both Kotlin and Java, and I do know and am writing Coroutines code. I simply enjoy writing in both languages and would love if they work together seamlessly in mixed codebases.

Ah, this is where the discussion gets... interesting: Java compatibility. I have specifically been avoiding this since I'm afraid it would bring about some very unproductive "arguments".
The reason 2.x is good for accessibility is because it doesn't discriminate between languages. In both Java and Kotlin the library works exactly the same way.

Proposal

If using GlobalScope is the problem, why not make the old API functions take a CoroutineScope as a parameter? You actually can obtain a CoroutineScope in Java:

LifecycleOwnerKt.getLifecycleScope(getViewLifecycleOwner()); //this is lifecycleScope!

Again, if you don't want to listen to me, it's fine, I can do it myself.

[wasky]

To call suspending function from Java code you can use this solution: https://stackoverflow.com/a/52887677/1219654

[unbiaseduser]

Oh ok, that's interesting. Still, I think things can be better documented.

Speaking of, why are the Java documentations removed?

Java has never been a dead language - it's constantly evolving (structured concurrency in Java is a thing now, although is in preview). Most Java 8+ language features can be used in any Android version via core library desugaring and Java language version can be set in a project like this:

compileOptions {
    sourceCompatibility JavaVersion.VERSION_19
    targetCompatibility JavaVersion.VERSION_19
}

That's it! You can use modern Java to develop Android apps! Say hello to enhanced switch syntax and streamlined instanceof pattern matching! No more custom SAM interfaces to perform simple actions - java.util.function.Consumer<T> and friends to the rescue! You can use them similar to gasp Kotlin functions! Functional programming has never been easier with java.util.stream.Stream<E> - fluent declarative APIs just like, holy crap, Kotlin extension functions on Iterables!

Many, MANY people do not realize that Java is much more powerful than, say, 2019 (when Kotlin was officially announced for Android dev). (And rightfully so - Java 7 anonymous classes boilerplate still haunts many long time devs to this day)

I'm joking about that last part, but it also delivers my point

A very unfortunate consequence is that everywhere, EVERYWHERE Android dev is concerned, people have extremely negative connotations about Java devs.

• oH My goD, thAt peRSoN useS jAva iN 2023, sUch A dInoSaUR! tRuly a HerO iN thE jUraSSic agE!!!11!1!1!
• wHy doNt yOu JusT usE kOtliN lOl?

You may realize that I'm very defensive and steadfast about accessibility. That's because I want to - no, that's my mission to prevent those toxic absolutisms (buT jAva iS juSt baD!!!) and make absolutely sure that technology is friendly to everyone and their cats and no one is discriminated just because they have their preferences about which tools they are comfortable with. Kotlin's interoperability with Java is top-notch, why can't people's mindsets also follow that as well?

[burhanrashid52]

@unbiaseduser
I understand your sentiment again java and I get your argument against not forcing users to use coroutines. However, PR #499 also fixes the long-standing issue #374. We tried various approaches in the past to fix it but it keeps coming back. and to me, the coroutine PR might finally fix it.

And the whole reason to migrate to Kotlin was to leverage all new features available in Kotlin including coroutines.

If you have a solution to fix this problem using the old API which uses AsyncTask then I am happy to keep both AsyncTask and Coroutine versions of the API. We'll change the name of the coroutine function to differentiate it better.

Let us know with your PR or else we keeping discussing this without any progress.

[unbiaseduser]

Let me clarify this: I... never said that I would like to keep the old version. The sentence at the beginning means that I am willing to make PRs to improve the new version, specifically the new API! I specifically said that the future versions are awesome and I specifically pointed out that, you know, maybe it could be more accessible.
I'm deeply sorry if any of my points was not conveyed properly.
I have already accepted the above solution (aka: I am fine with leaving the new version as-is) and the last message is specifically about the documentation (or the removal of it), in which I explained why both Java and Kotlin usage should be documented equally (even more so in Java case since the only way to use the non-deprecated API is through the above solution).

As for what I can do: Like I said before, I can fill in the gaps myself (specifically: the documentation). I just need someone to OK me so that I can safely do so (since you still have the right to straight up deny me). I never thought that everyone just... goes dead silent for days and now everyone's mad at me for not making progress!

[burhanrashid52]

Let me clarify this: I... never said that I would like to keep the old version. The sentence at the beginning means that I am willing to make PRs to improve the new version, specifically the new API!

Oh!! I thought it was about keeping the old APIs to support the java version. My bad.

I have already accepted the above solution (aka: I am fine with leaving the new version as-is)

Cool than. I will review the PR and merge it if it looks good.

I can fill in the gaps myself (specifically: the documentation). I just need someone to OK me so that I can safely do so

Cool. Can you create a new issue with the description you are planning to make for the documentation? We can move the discussion there.

[wasky]

New API for Java users introduced in #512

[unbiaseduser]

Oh thank you! So, will my documentation effort be needed? I mean, I already wrote a draft of the instructions on how to call suspend functions here
suspend_fun_tutorial_draft.md

Also, a little nitpick: Java API has a optional parameter yet isn't annotated @JvmOverloads, so Java callers will always have to supply all params

[github-actions]

This issue is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 4 days.

[unbiaseduser]

I'll close this now, thank you