Respond to review comments

Author: samtstern

database/src/androidTest/java/com/firebase/ui/database/Bean.java

@@ -32,14 +32,14 @@ public boolean isBool() {
     }
 
     public void setNumber(int number) {
-        this.mNumber = number;
+        mNumber = number;
     }
 
     public void setText(String text) {
-        this.mText = text;
+        mText = text;
     }
 
     public void setBool(boolean bool) {
-        this.mBool = bool;
+        mBool = bool;
     }
 }

database/src/androidTest/java/com/firebase/ui/database/FirebaseArrayOfObjectsTest.java

@@ -46,7 +46,7 @@ public void setUp() throws Exception {
                 .getReference()
                 .child("firebasearray")
                 .child("objects");
-        mArray = new FirebaseArray<>(mRef, new ClassSnapshotParser<>(Bean.class));
+        mArray = new FirebaseArray<>(mRef, Bean.class);
         mRef.removeValue();
         mListener = runAndWaitUntil(mArray, new Runnable() {
             @Override

database/src/androidTest/java/com/firebase/ui/database/FirebaseIndexArrayOfObjectsTest.java

@@ -46,7 +46,7 @@ public void setUp() throws Exception {
         mRef = databaseInstance.getReference().child("firebasearray").child("objects");
         mKeyRef = databaseInstance.getReference().child("firebaseindexarray").child("objects");
 
-        mArray = new FirebaseIndexArray<>(mKeyRef, mRef, new ClassSnapshotParser<>(Bean.class));
+        mArray = new FirebaseIndexArray<>(mKeyRef, mRef, Bean.class);
         mRef.removeValue();
         mKeyRef.removeValue();
 

database/src/main/java/com/firebase/ui/database/CachingObservableSnapshotArray.java

@@ -1,5 +1,7 @@
 package com.firebase.ui.database;
 
+import android.support.annotation.NonNull;
+
 import com.google.firebase.database.DataSnapshot;
 
 import java.util.HashMap;
@@ -14,11 +16,24 @@ public abstract class CachingObservableSnapshotArray<T> extends ObservableSnapsh
 
     private Map<String, T> mObjectCache = new HashMap<>();
 
+    /**
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray()}.
+     */
     public CachingObservableSnapshotArray() {
         super();
     }
 
-    public CachingObservableSnapshotArray(SnapshotParser<T> parser) {
+    /**
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(Class)}.
+     */
+    public CachingObservableSnapshotArray(@NonNull Class<T> tClass) {
+        super(tClass);
+    }
+
+    /**
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(SnapshotParser)}.
+     */
+    public CachingObservableSnapshotArray(@NonNull SnapshotParser<T> parser) {
         super(parser);
     }
 

database/src/androidTest/java/com/firebase/ui/database/ClassSnapshotParser.java renamed to database/src/main/java/com/firebase/ui/database/ClassSnapshotParser.java

@@ -23,31 +23,52 @@
 import com.google.firebase.database.ValueEventListener;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 /**
  * This class implements a collection on top of a Firebase location.
  */
 public class FirebaseArray<T> extends CachingObservableSnapshotArray<T> implements ChildEventListener, ValueEventListener {
 
-    protected final Query mQuery;
+    private Query mQuery;
     private List<DataSnapshot> mSnapshots = new ArrayList<>();
 
-    private Map<String, T> mObjectCache = new HashMap<>();
-
     /**
+     * Create a new FirebaseArray with no {@link SnapshotParser}. Calls to
+     * {@link #getObject(int)} will fail.
+     *
      * @param query The Firebase location to watch for data changes. Can also be a slice of a
      *              location, using some combination of {@code limit()}, {@code startAt()}, and
      *              {@code endAt()}.
      */
     public FirebaseArray(Query query) {
-        this(query, null);
+        super();
+        init(query);
+    }
+
+    /**
+     * Create a new FirebaseArray that parses snapshots as members of a given class.
+     *
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(Class)}.
+     * See {@link #FirebaseArray(Query)}.
+     */
+    public FirebaseArray(Query query, Class<T> tClass) {
+        super(tClass);
+        init(query);
     }
 
+    /**
+     * Create a new FirebaseArray with a custom {@link SnapshotParser}.
+     *
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(SnapshotParser)}.
+     * See {@link #FirebaseArray(Query)}.
+     */
     public FirebaseArray(Query query, SnapshotParser<T> parser) {
         super(parser);
+        init(query);
+    }
+
+    private void init(Query query) {
         mQuery = query;
     }
 

database/src/main/java/com/firebase/ui/database/FirebaseArray.java

@@ -38,22 +38,44 @@ public class FirebaseIndexArray<T> extends CachingObservableSnapshotArray<T> imp
     private FirebaseArray<String> mKeySnapshots;
     private List<DataSnapshot> mDataSnapshots = new ArrayList<>();
 
-    private Map<String, T> mObjectCache = new HashMap<>();
-
     /**
+     * Create a new FirebaseIndexArray without a {@link SnapshotParser}.
+     * Calls to {@link #getObject(int)} will fail.
+     *
      * @param keyQuery The Firebase location containing the list of keys to be found in {@code
      *                 dataRef}. Can also be a slice of a location, using some combination of {@code
      *                 limit()}, {@code startAt()}, and {@code endAt()}.
      * @param dataRef  The Firebase location to watch for data changes. Each key key found at {@code
      *                 keyQuery}'s location represents a list item in the {@link RecyclerView}.
      */
     public FirebaseIndexArray(Query keyQuery, DatabaseReference dataRef) {
-        this(keyQuery, dataRef, null);
+        super();
+        init(keyQuery, dataRef);
+    }
+
+    /**
+     * Create a new FirebaseIndexArray that parses snapshots as members of a given class.
+     *
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(Class)}.
+     * See {@link #FirebaseIndexArray(Query, DatabaseReference)}.
+     */
+    public FirebaseIndexArray(Query keyQuery, DatabaseReference dataRef, Class<T> tClass) {
+        super(tClass);
+        init(keyQuery, dataRef);
     }
 
+    /**
+     * Create a new FirebaseIndexArray with a custom {@link SnapshotParser}.
+     *
+     * See {@link ObservableSnapshotArray#ObservableSnapshotArray(SnapshotParser)}.
+     * See {@link #FirebaseIndexArray(Query, DatabaseReference)}.
+     */
     public FirebaseIndexArray(Query keyQuery, DatabaseReference dataRef, SnapshotParser<T> parser) {
         super(parser);
+        init(keyQuery, dataRef);
+    }
 
+    private void init(Query keyQuery, DatabaseReference dataRef) {
         mKeySnapshots = new FirebaseArray<>(keyQuery, new SnapshotParser<String>() {
             @Override
             public String parseSnapshot(DataSnapshot snapshot) {
@@ -193,7 +215,7 @@ public void onDataChange(DataSnapshot snapshot) {
                     removeData(index);
                     notifyChangeEventListeners(ChangeEventListener.EventType.REMOVED, snapshot, index);
                 } else {
-                    // Data we never knew about has disappeared
+                    // Data does not exist
                     Log.w(TAG, "Key not found at ref: " + snapshot.getRef());
                 }
             }

database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

@@ -45,12 +45,8 @@ public FirebaseListAdapter(Activity activity,
                                ObservableSnapshotArray<T> snapshots,
                                Class<T> modelClass,
                                @LayoutRes int modelLayout) {
-        mSnapshots = snapshots;
-        mActivity = activity;
-        mModelClass = modelClass;
-        mLayout = modelLayout;
 
-        startListening();
+        init(activity, snapshots, modelClass, modelLayout);
     }
 
     /**
@@ -63,8 +59,14 @@ public FirebaseListAdapter(Activity activity,
                                Class<T> modelClass,
                                @LayoutRes int modelLayout,
                                Query query) {
-        mSnapshots = new FirebaseArray<T>(query, this);
+
+        init(activity, new FirebaseArray<T>(query, this), modelClass, modelLayout);
+    }
+
+    private void init(Activity activity, ObservableSnapshotArray<T> snapshots,
+                      Class<T> modelClass, @LayoutRes int modelLayout) {
         mActivity = activity;
+        mSnapshots = snapshots;
         mModelClass = modelClass;
         mLayout = modelLayout;
 

database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java

@@ -51,12 +51,8 @@ public FirebaseRecyclerAdapter(ObservableSnapshotArray<T> snapshots,
                                    Class<T> modelClass,
                                    @LayoutRes int modelLayout,
                                    Class<VH> viewHolderClass) {
-        mSnapshots = snapshots;
-        mModelClass = modelClass;
-        mViewHolderClass = viewHolderClass;
-        mModelLayout = modelLayout;
 
-        startListening();
+        init(snapshots, modelClass, modelLayout, viewHolderClass);
     }
 
     /**
@@ -69,7 +65,13 @@ public FirebaseRecyclerAdapter(Class<T> modelClass,
                                    @LayoutRes int modelLayout,
                                    Class<VH> viewHolderClass,
                                    Query query) {
-        mSnapshots = new FirebaseArray<T>(query, this);
+
+        init(new FirebaseArray<>(query, this), modelClass, modelLayout, viewHolderClass);
+    }
+
+    private void init(ObservableSnapshotArray<T> snapshots, Class<T> modelClass,
+                      @LayoutRes int modelLayout, Class<VH> viewHolderClass) {
+        mSnapshots = snapshots;
         mModelClass = modelClass;
         mViewHolderClass = viewHolderClass;
         mModelLayout = modelLayout;

database/src/main/java/com/firebase/ui/database/FirebaseRecyclerAdapter.java

@@ -24,11 +24,28 @@ public abstract class ObservableSnapshotArray<T> extends ImmutableList<DataSnaps
     protected final List<ChangeEventListener> mListeners = new CopyOnWriteArrayList<>();
     protected final SnapshotParser<T> mParser;
 
+    /**
+     * Create an ObservableSnapshotArray without a {@link SnapshotParser}. Calls to
+     * {@link #getObject(int)} will fail.
+     */
     public ObservableSnapshotArray() {
         mParser = null;
     }
 
-    public ObservableSnapshotArray(SnapshotParser<T> parser) {
+    /**
+     * Create an ObservableSnapshotArray where snapshots are parsed as objects of a particular
+     * class.  See {@link ClassSnapshotParser}.
+     * @param tClass the class as which DataSnapshots should be parsed.
+     */
+    public ObservableSnapshotArray(@NonNull Class<T> tClass) {
+        this(new ClassSnapshotParser<>(tClass));
+    }
+
+    /**
+     * Create an ObservableSnapshotArray with a custom {@link SnapshotParser}.
+     * @param parser the {@link SnapshotParser to use}.
+     */
+    public ObservableSnapshotArray(@NonNull SnapshotParser<T> parser) {
         mParser = parser;
     }