Avoid double API call from bottom navigation fast tap

Question

I developed an Android app using Kotlin and all the structure and functionality are complete, but I noticed a small issue when I tap fast repeatedly, at least twice on a button that does an API call.

For the API calls I am using a combination of RetroFit2 and GsonConverterFactory. The call is as follows:

    fun fetchInfo(id: Int) {
        val retrofit = Retrofit.Builder()
            .baseUrl("https://www.mysitesurl.com/api/")
            .addConverterFactory(GsonConverterFactory.create())
            .build()

        val api = retrofit.create(ApiService::class.java)

        api.getInfo(id).enqueue(object: Callback<DataType> {

            override fun onResponse(call: Call<DataType>, response: Response<DataType>) {
                var resp = response.body()!!
                my_image.setImageResource(resources.getIdentifier(resp.image, "drawable", context!!.packageName))
                my_image.visibility = View.VISIBLE

                my_label.text = resp.text
                my_label.visibility = View.VISIBLE
            }

            override fun onFailure(call: Call<FechaDia>, t: Throwable) {

            }

        })
    }

I have edited the code a bit to avoid specific variable names

So, as mentioned before this code works fine, the problem comes when I click the navigation button twice fast. From what I understand it tries to make another API call before the current one has responded and gets a null response, so I basically try to substitute an image with a null resource and it shows me this error:

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.ImageView.setImageResource(int)' on a null object reference`

I tried using try/catch but it still makes the call and still receives a null request. Is there a way to block this from happening or what am I missing in my process here?

The main issue is that it doesn't just show an error, the app closes and shows the App has stopped. Open app again message.

You are trying to force a nullable `response.body()!!` to be non-null so try to do something like `response.body()?.run { // code goes here }`. — Rodrigo Queiroz, Jun 27 '19 at 16:42
Just to confirm @RodrigoQueiroz, if I do that, all the UI setters would run only when the response is not null? Otherwise it would do nothing? — Mihail Minkov, Jun 27 '19 at 16:51
Indeed, nothing would be set unless you have an actual body! I don't think using a flag is a viable solution so if there was more code or context maybe a better solution could have been proposed! — Rodrigo Queiroz, Jun 27 '19 at 16:54
@RodrigoQueiroz your solution did the trick without modifying too much code, you can put it as an answer :) — Mihail Minkov, Jun 27 '19 at 17:41
Just one more thing, it works fine for regular fragments, but sometimes the app still closes when I am using adapters. What I did is I put the adapter assignation inside the run block. Is that correct or does it differ in that case? — Mihail Minkov, Jun 27 '19 at 17:54

score 1 · Answer 1 · edited Jun 28 '19 at 00:54

1

Use a global flag like this:

private boolean clicked = false;

In onClick:

if(false){
     callApi();
     clicked = true;
}

And in the Success or Error response, make it false:

clicked = false;

edited Jun 28 '19 at 00:54

Gino Mempin

25,369
29
96
135

answered Jun 27 '19 at 16:32

Anupam

2,845
2
16
30

The issue is that that happens with my bottom navigation and it affects the main fragment, so you are saying that I make a global variable in my main activity and change it? What happens when there are local (fragment specific) buttons that also make API calls? – Mihail Minkov Jun 27 '19 at 16:36
Make the flag in activity and change it from everywhere(Activity as well as the fragments) – Anupam Jun 27 '19 at 16:39

Rodrigo Queiroz · Accepted Answer · 2019-06-27T19:00:49.387

As mentioned in the comments, try not to force nullable to non-nullable type as there will be side effects (Exceptions).

Ideally, you would want to also decouple things a bit for better code readability:

fun retrofit(): Retrofit = Retrofit.Builder()
    .baseUrl("https://www.mysitesurl.com/api/")
    .addConverterFactory(GsonConverterFactory.create())
    .build()

fun apiService(): ApiService = retrofit().create(ApiService::class.java)

fun fetchInfo(id: Int) =
    apiService().getInfo(id).enqueue(object : Callback<Element.DataType> {
        override fun onResponse(
            call: Call<Element.DataType>,
            response: Response<Element.DataType>
        ) {
            response.body()?.run { renderView(this) }
        }
        override fun onFailure(call: Call<FechaDia>, t: Throwable) {}
    })

fun renderView(response: DataType) = view?.apply {
    val image = resources.getIdentifier(response.image, "drawable", context.packageName))
    my_image.setImageResource(image)
    my_image.visibility = View.VISIBLE
    my_label.text = response.text
    my_label.visibility = View.VISIBLE
}

If you have an adapter you don't have to assign it in the renderView as what you'd probably do is only update the adapter once you get data back from the API.

Have the adapter as property either in the Activity or in Fragment then once you get the response call the adapter and send the list.

To complement the question as there isn't much code to look at, I suppose if you are using the bottom navigation without using the jetpack navigation library you could use the bottom navigation with ViewPager and use an OnNavigationItemSelectedListener on the bottom navigation to switch between pages on the ViewPager adapter.

And if you had retainInstance = true the fragment wouldn't re-create therefore only one call to the API would have been made.

Your solution worked for me, I didn't divide it as much as you suggest, but I did use the `response.body()?.run { ... }` :) — Mihail Minkov, Jul 02 '19 at 18:19

score 0 · Answer 3 · answered Jun 27 '19 at 18:29

I feel like this is a great situation for a debouncer.

Debouncing is a pattern that prevents multiple signals for the same function to be fired too quickly in succession and will only fire once if pressed within the given time frame.

First reference I found: Kotlin Android debounce

I would suggest looking at SANAT's answer, this looks to be a very clean implementation, and will help you handle multiple clicks without having multiple function fires.

Avoid double API call from bottom navigation fast tap

3 Answers3