Skip to content

Update database access modifiers #297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Nov 29, 2016
Merged

Update database access modifiers #297

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0deadf8
Update access modifiers
SUPERCILEX Sep 10, 2016
329a5dc
Update FirebaseListAdapter.java
SUPERCILEX Sep 12, 2016
81b7da9
Update FirebaseRecyclerAdapter.java
SUPERCILEX Sep 12, 2016
fa82073
Merge pull request #15 from firebase/master
SUPERCILEX Sep 24, 2016
4b6550c
Merge branch 'firebase-master' into update-database-access-modifiers
SUPERCILEX Oct 4, 2016
12760c1
Cleanup
SUPERCILEX Oct 4, 2016
ccea6ec
Cleanup
SUPERCILEX Oct 4, 2016
b818cf8
Cleanup
SUPERCILEX Oct 4, 2016
7033d77
Cleanup
SUPERCILEX Oct 4, 2016
14d1f8c
Update FirebaseArray.java
SUPERCILEX Oct 4, 2016
d81994b
Update FirebaseArray.java
SUPERCILEX Oct 4, 2016
3b90228
Update FirebaseArray.java
SUPERCILEX Oct 4, 2016
1ead7c3
Update FirebaseArray.java
SUPERCILEX Oct 4, 2016
8ff8c06
Revert
SUPERCILEX Oct 8, 2016
d6fae5a
Merge remote-tracking branch 'firebase/version-1.0.0-dev' into update…
SUPERCILEX Nov 2, 2016
f4f031b
Update access modifiers
SUPERCILEX Nov 2, 2016
9c5f92b
Cleanup
SUPERCILEX Nov 2, 2016
00f389e
More updated access modifiers
SUPERCILEX Nov 2, 2016
d2cec9d
More updated access modifiers
SUPERCILEX Nov 2, 2016
22baf3d
Cleanup
SUPERCILEX Nov 2, 2016
8aeaa7c
Cleanup
SUPERCILEX Nov 2, 2016
81acc87
Merge branch 'master' into update-database-access-modifiers
SUPERCILEX Nov 24, 2016
e5ca921
Merge remote-tracking branch 'firebase/master' into update-database-a…
SUPERCILEX Nov 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions database/src/main/java/com/firebase/ui/database/FirebaseArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
* This class implements an array-like collection on top of a Firebase location.
*/
class FirebaseArray implements ChildEventListener {
Copy link
Collaborator Author

@SUPERCILEX SUPERCILEX Sep 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed everything below to be no modifier since the class itself shouldn't be accessed from anywhere outside of the package, I don't see why the methods inside shouldn't follow along.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of SOLID programming, shouldn't at least the class itself be at least public for it to be open to extension? Building another adapter as an add-on to FirebaseUI using FirebaseArray wouldn't be possible with this configuration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think about that. Thanks! I'll update this PR when I get home.

public interface OnChangedListener {
interface OnChangedListener {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should definitely be changed, I don't know why it's not at least protected.

enum EventType { Added, Changed, Removed, Moved }
void onChanged(EventType type, int index, int oldIndex);
void onCancelled(DatabaseError databaseError);
Expand All @@ -35,21 +35,21 @@ enum EventType { Added, Changed, Removed, Moved }
private OnChangedListener mListener;
private ArrayList<DataSnapshot> mSnapshots;

public FirebaseArray(Query ref) {
FirebaseArray(Query ref) {
mQuery = ref;
mSnapshots = new ArrayList<DataSnapshot>();
mQuery.addChildEventListener(this);
}

public void cleanup() {
void cleanup() {
mQuery.removeEventListener(this);
}

public int getCount() {
int getCount() {
return mSnapshots.size();

}
public DataSnapshot getItem(int index) {
DataSnapshot getItem(int index) {
return mSnapshots.get(index);
}

Expand Down Expand Up @@ -100,21 +100,21 @@ public void onCancelled(DatabaseError databaseError) {
}
// End of ChildEventListener methods

public void setOnChangedListener(OnChangedListener listener) {
void setOnChangedListener(OnChangedListener listener) {
mListener = listener;
}

protected void notifyChangedListeners(OnChangedListener.EventType type, int index) {
void notifyChangedListeners(OnChangedListener.EventType type, int index) {
notifyChangedListeners(type, index, -1);
}

protected void notifyChangedListeners(OnChangedListener.EventType type, int index, int oldIndex) {
void notifyChangedListeners(OnChangedListener.EventType type, int index, int oldIndex) {
if (mListener != null) {
mListener.onChanged(type, index, oldIndex);
}
}

protected void notifyCancelledListeners(DatabaseError databaseError) {
void notifyCancelledListeners(DatabaseError databaseError) {
if (mListener != null) {
mListener.onCancelled(databaseError);
}
Expand Down
6 changes: 3 additions & 3 deletions database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@
*/
public abstract class FirebaseListAdapter<T> extends BaseAdapter {

private Activity mActivity;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had some cases in the past where I wanted to use the activity and layout in my concrete implementation. I'm not sure if these still apply, but would prefer to keep these protected for now.

private final Class<T> mModelClass;
protected int mLayout;
protected Activity mActivity;
FirebaseArray mSnapshots;
private int mLayout;
private FirebaseArray mSnapshots;


/**
Expand Down
8 changes: 4 additions & 4 deletions database/src/main/java/com/firebase/ui/database/FirebaseRecyclerAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@
*/
public abstract class FirebaseRecyclerAdapter<T, VH extends RecyclerView.ViewHolder> extends RecyclerView.Adapter<VH> {

Class<T> mModelClass;
protected int mModelLayout;
Class<VH> mViewHolderClass;
FirebaseArray mSnapshots;
private Class<T> mModelClass;
private int mModelLayout;
private Class<VH> mViewHolderClass;
private FirebaseArray mSnapshots;

/**
* @param modelClass Firebase will marshall the data at a location into an instance of a class that you provide
Expand Down