From 762df274f96d6e4e176b0ee212fc2b508b5f247c Mon Sep 17 00:00:00 2001 From: Adam Copp Date: Tue, 28 Aug 2012 11:24:44 +0100 Subject: Scope changes and removal of unnessecary variable Adds isFullScreen() and setPhotoIndex() protected methods to PhotoViewActivity in order to allow subclasses access to this information. Removes mDataValid from BaseCursorPagerAdapter since it was hiding how simple the checks actually were (it was just mCursor != null) A nullcheck has also been added to BaseFragmentPagerAdapter in instantiateItem. in the long term, BaseCursorPagerAdapter should not be so fragile, and should not surface nulls, but that change could be quite involved, so should probably not be rolled into this change. Change-Id: I4889c5ae2a764660926c70da25b85565f4600188 --- .../src/com/android/ex/photo/PhotoViewActivity.java | 10 +++++++++- .../ex/photo/adapters/BaseCursorPagerAdapter.java | 20 +++++++++----------- .../ex/photo/adapters/BaseFragmentPagerAdapter.java | 4 ++++ 3 files changed, 22 insertions(+), 12 deletions(-) (limited to 'photoviewer') diff --git a/photoviewer/src/com/android/ex/photo/PhotoViewActivity.java b/photoviewer/src/com/android/ex/photo/PhotoViewActivity.java index 8628331..ff3eb4c 100644 --- a/photoviewer/src/com/android/ex/photo/PhotoViewActivity.java +++ b/photoviewer/src/com/android/ex/photo/PhotoViewActivity.java @@ -415,7 +415,7 @@ public class PhotoViewActivity extends Activity implements /** * Updates the title bar according to the value of {@link #mFullScreen}. */ - private void setFullScreen(boolean fullScreen, boolean setDelayedRunnable) { + protected void setFullScreen(boolean fullScreen, boolean setDelayedRunnable) { final boolean fullScreenChanged = (fullScreen != mFullScreen); mFullScreen = fullScreen; @@ -549,4 +549,12 @@ public class PhotoViewActivity extends Activity implements postActionBarHideRunnableWithDelay(); } } + + protected boolean isFullScreen() { + return mFullScreen; + } + + protected void setPhotoIndex(int index) { + mPhotoIndex = index; + } } diff --git a/photoviewer/src/com/android/ex/photo/adapters/BaseCursorPagerAdapter.java b/photoviewer/src/com/android/ex/photo/adapters/BaseCursorPagerAdapter.java index 848d79a..f827690 100644 --- a/photoviewer/src/com/android/ex/photo/adapters/BaseCursorPagerAdapter.java +++ b/photoviewer/src/com/android/ex/photo/adapters/BaseCursorPagerAdapter.java @@ -38,7 +38,6 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { private static final String TAG = "BaseCursorPagerAdapter"; Context mContext; - private boolean mDataValid; private Cursor mCursor; private int mRowIDColumn; /** Mapping of row ID to cursor position */ @@ -67,9 +66,11 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { */ public abstract Fragment getItem(Context context, Cursor cursor, int position); + // TODO: This shouldn't just return null - maybe it needs to wait for a cursor to be supplied? + // See b/7103023 @Override public Fragment getItem(int position) { - if (mDataValid && moveCursorTo(position)) { + if (mCursor != null && moveCursorTo(position)) { return getItem(mContext, mCursor, position); } return null; @@ -77,7 +78,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { @Override public int getCount() { - if (mDataValid && mCursor != null) { + if (mCursor != null) { return mCursor.getCount(); } else { return 0; @@ -86,7 +87,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { @Override public Object instantiateItem(View container, int position) { - if (!mDataValid) { + if (mCursor == null) { throw new IllegalStateException("this should only be called when the cursor is valid"); } @@ -127,7 +128,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { * @return true if data is valid */ public boolean isDataValid() { - return mDataValid; + return mCursor != null; } /** @@ -141,7 +142,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { * Returns the data item associated with the specified position in the data set. */ public Object getDataItem(int position) { - if (mDataValid && moveCursorTo(position)) { + if (mCursor != null && moveCursorTo(position)) { return mCursor; } else { return null; @@ -152,7 +153,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { * Returns the row id associated with the specified position in the list. */ public long getItemId(int position) { - if (mDataValid && moveCursorTo(position)) { + if (mCursor != null && moveCursorTo(position)) { return mCursor.getString(mRowIDColumn).hashCode(); } else { return 0; @@ -180,10 +181,8 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { mCursor = newCursor; if (newCursor != null) { mRowIDColumn = newCursor.getColumnIndex(PhotoContract.PhotoViewColumns.URI); - mDataValid = true; } else { mRowIDColumn = -1; - mDataValid = false; } setItemPosition(); @@ -231,7 +230,6 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { private void init(Context context, Cursor c) { boolean cursorPresent = c != null; mCursor = c; - mDataValid = cursorPresent; mContext = context; mRowIDColumn = cursorPresent ? mCursor.getColumnIndex(PhotoContract.PhotoViewColumns.URI) : -1; @@ -242,7 +240,7 @@ public abstract class BaseCursorPagerAdapter extends BaseFragmentPagerAdapter { * row id to cursor position. */ private void setItemPosition() { - if (!mDataValid || mCursor == null || mCursor.isClosed()) { + if (mCursor == null || mCursor.isClosed()) { mItemPosition = null; return; } diff --git a/photoviewer/src/com/android/ex/photo/adapters/BaseFragmentPagerAdapter.java b/photoviewer/src/com/android/ex/photo/adapters/BaseFragmentPagerAdapter.java index 9c24575..2065b2a 100644 --- a/photoviewer/src/com/android/ex/photo/adapters/BaseFragmentPagerAdapter.java +++ b/photoviewer/src/com/android/ex/photo/adapters/BaseFragmentPagerAdapter.java @@ -86,6 +86,10 @@ public abstract class BaseFragmentPagerAdapter extends PagerAdapter { mCurTransaction.attach(fragment); } else { fragment = getItem(position); + if(fragment == null) { + if (DEBUG) Log.e(TAG, "NPE workaround for getItem(). See b/7103023"); + return null; + } if (DEBUG) Log.v(TAG, "Adding item #" + position + ": f=" + fragment); mCurTransaction.add(container.getId(), fragment, makeFragmentName(container.getId(), position)); -- cgit v1.2.3