turing-tech / MaterialScrollBar

An Android library that brings the Material Design 5.1 sidebar to pre-5.1 devices.
Apache License 2.0
778 stars 126 forks source link

fixed empty data crash when indicator is shown #125 #126

Closed androideveloper closed 4 years ago

androideveloper commented 5 years ago

This happened after update to latest version, cause method @Override public Character getCharacterForElement(int element) { } receives index 0 instead of -1 in earlier versions, when data is empty. Per Java implementation used in Android in case of 0, empty list throws IndexOutOfBoundsException instead of ArrayIndexOutOfBoundsException

    /**
     * Returns the element at the specified position in this list.
     *
     * @param  index index of the element to return
     * @return the element at the specified position in this list
     * @throws IndexOutOfBoundsException {@inheritDoc}
     */
   public E get(int index) {
        if (index >= size)
            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));

        return (E) elementData[index];
    }
turing-tech commented 5 years ago

It's unclear to me what exactly the error is and why this PR would fix it. Could you provide more detail?

androideveloper commented 5 years ago

Our implementation of getCharacterForElement is

    @Override
    public Character getCharacterForElement(int position) {
                String code = items.get(position).getCode();
                return TextUtils.isEmpty(code) ? ' ' : code.charAt(0);
    }

After upgrade to latest version from v13.2.3, this code leads to crash. Before the upgrade in empty list case, position received as parameter was -1 and exception in that case was ArrayIndexOutOfBoundsException, which you handle. After upgrade position passed in case of empty list is 0, which leads to IndexOutOfBoundsException, which you don't handle

androideveloper commented 5 years ago

IndexOutOfBoundsException is parent class of ArrayIndexOutOfBoundsException, so we should handle it

androideveloper commented 5 years ago

Any updates?

turing-tech commented 5 years ago

getCharacterForElement should probably not be getting any calls when the RV is empty. I'll need to test things out and get back to you. I'm unfortunately swamped with university work, but I'll work to get this done as soon as I can manage.

androideveloper commented 5 years ago

That's another question, but in this context suggested solution works pretty well. Ok, will wait for your response.

turing-tech commented 5 years ago

In what circumstance is the indicator being shown that's causing the error other than the list being empty? Could you provide the stacktrace?

androideveloper commented 5 years ago
2019-03-12 09:44:47.612 3356-3356/com.armsoft.mtrade.dev E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.armsoft.mtrade.dev, PID: 3356
    java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
        at java.util.ArrayList.get(ArrayList.java:437)
        at com.armsoft.mtrade.ui.directory.item.ItemsAdapter.getCharacterForElement(ItemsAdapter.java:81)
        at com.turingtechnologies.materialscrollbar.AlphabetIndicator.getTextElement(AlphabetIndicator.java:34)
        at com.turingtechnologies.materialscrollbar.AlphabetIndicator.getTextElement(AlphabetIndicator.java:25)
        at com.turingtechnologies.materialscrollbar.Indicator.setText(Indicator.java:141)
        at com.turingtechnologies.materialscrollbar.ScrollingUtilities.scrollHandleAndIndicator(ScrollingUtilities.java:73)
        at com.turingtechnologies.materialscrollbar.MaterialScrollBar.onLayout(MaterialScrollBar.java:309)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.RelativeLayout.onLayout(RelativeLayout.java:1083)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at androidx.drawerlayout.widget.DrawerLayout.onLayout(DrawerLayout.java:1231)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at androidx.slidingpanelayout.widget.SlidingPaneLayout.onLayout(SlidingPaneLayout.java:710)
        at com.mikepenz.crossfader.view.CrossFadeSlidingPaneLayout.onLayout(CrossFadeSlidingPaneLayout.java:101)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
2019-03-12 09:44:47.613 3356-3356/com.armsoft.mtrade.dev E/AndroidRuntime:     at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at com.android.internal.policy.DecorView.onLayout(DecorView.java:753)
        at android.view.View.layout(View.java:20684)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2801)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2328)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1468)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:7208)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1004)
        at android.view.Choreographer.doCallbacks(Choreographer.java:816)
        at android.view.Choreographer.doFrame(Choreographer.java:751)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:990)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6694)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
androideveloper commented 5 years ago

Any updates?

androideveloper commented 5 years ago

Hi @turing-tech. It has been a long time since you last updated on this issue. We're still waiting for merge or any other solution.

khecha commented 4 years ago

@androideveloper Hi, I am unfortunately experiencing the same problem. Did you or @turing-tech find a solution?

Great library btw!

androideveloper commented 4 years ago

@khecha This PR should resolve the issue when merged

khecha commented 4 years ago

@khecha This PR should resolve the issue when merged

Thanks, I hope it can be merged soon.

Do you have any quick fixes while I wait for the merge?

androideveloper commented 4 years ago

@khecha I don't think so, it's already a year since I've opened the PR. You can just catch that exception in your code until fixed.

karapetyanmery commented 4 years ago

Hi @turing-tech. I am experiencing the same problem, and waiting for the PR to merge. When can we have update on this issue?

turing-tech commented 4 years ago

Merged