Animated ScrollView

Question

I have this code which runs when an Async HTTP call is made on a HorizontalScrollView

            //These 3 defined outside of the method
            ObjectAnimator animatorR = null;
            ObjectAnimator animatorL = null;
            boolean loop = true;

            Inside the method
            if (animatorL != null) animatorL.cancel();
            animatorL = null;
            if (animatorR != null) animatorR.cancel();
            animatorR = null;
            HorizontalScrollView sv = (HorizontalScrollView) findViewById(R.id.scroll_view);
            loop = true;
            sv.setOnTouchListener(new View.OnTouchListener() {
                @Override
                public boolean onTouch(View v, MotionEvent event) {
                    animatorL.cancel();
                    animatorR.cancel();
                    loop = false;
                    sv.setOnTouchListener(null);
                    return false;
                }
            });
            animatorR = ObjectAnimator.ofInt(sv, "scrollX", sv.getMaxScrollAmount()).setDuration(4000);
            animatorL = ObjectAnimator.ofInt(sv, "scrollX", 0).setDuration(4000);

            animatorR.removeAllListeners();
            animatorR.addListener(new Animator.AnimatorListener() {
                @Override
                public void onAnimationStart(Animator animation) {

                }

                @Override
                public void onAnimationEnd(Animator animation) {
                    Log.d("AnimR", "Ended");
                    if (loop && animatorL != null) animatorL.start();
                }

                @Override
                public void onAnimationCancel(Animator animation) {

                }

                @Override
                public void onAnimationRepeat(Animator animation) {

                }
            });
            animatorR.start();

            animatorL.removeAllListeners();
            animatorL.addListener(new Animator.AnimatorListener() {
                @Override
                public void onAnimationStart(Animator animation) {

                }

                @Override
                public void onAnimationEnd(Animator animation) {
                    Log.d("AnimL", "Ended");
                    if (loop && animatorR != null) animatorR.start();
                }

                @Override
                public void onAnimationCancel(Animator animation) {

                }

                @Override
                public void onAnimationRepeat(Animator animation) {

                }
            });
        }

This is my first time using Animators and the code seems very clunky. Where could I make improvements to this code?

Answer 1

The biggest change I can see here would be to extract a class out of the two anonymous inner classes. The two variations are exactly the same except for which ObjectAnimator they should call next, and which logging tag to use. So let's extract a class and pass those values to its constructor: (Place the code below inside the same class as you are using right now)

private class MyAnimListener implements Animator.AnimatorListener {
    private ObjectAnimator  other;
    private final String    logMessage;

    public MyAnimListener(String logMessage, ObjectAnimator startOther) {
        this.logMessage = logMessage;
        this.other = startOther;
    }

    @Override
    public void onAnimationStart(Animator animation) {
    }

    @Override
    public void onAnimationEnd(Animator animation) {
        Log.d(logMessage, "Ended");
        if (loop && other != null) 
            other.start();
    }

    @Override
    public void onAnimationCancel(Animator animation) {
    }

    @Override
    public void onAnimationRepeat(Animator animation) {
    }
}       

You could then use this code to add the listeners:

animatorR.addListener(new MyAnimListener("AnimR", animatorL));
animatorL.addListener(new MyAnimListener("AnimL", animatorR));

However, if your two animator instances often switch to/from null, I would think about what it actually is that you are trying to achieve. (Honestly I'm not sure what you are trying to do, which can make this a bit of an X-Y problem).

If you really really want the two animator instances to switch to/from null, then you will have to use two variables for the MyAnimListener and modify its ObjectAnimator other field, something like this:

    // Variable declarations
animRListener = new MyAnimListener("AnimR", animatorL);
animatorR.addListener(animRListener);
animLListener = new MyAnimListener("AnimL", animatorR);
animatorL.addListener(animLListener);

// Later on
animatorR = null;
animLListener.other = null;

Besides this, I actually think your code might be cleaner than it looks, if that makes any sense...

---

I would recommend using the same tag parameter for all Log calls throughout your entire map. The Logcat utility can filter on a specific tag, by using it like this:

adb -d logcat YourTag:D YourOtherTag:D System.err:W *:S

I usually use something like this to be informed about both my own tags, stack traces from applications gone mad, and some other more or less useful Android messages:

adb -d logcat MyTag:D MyOtherTag:D MyThirdTag:D dalvikvm:I AndroidRuntime:E ResourceType:W PackageManager:I System.err:W *:S