0

If there's something I'm struggling with it's how to write good looking code with asynchronous calls to Firebase. Below is the code for one of my functions that retrieves all users and the current user from Firebase and checks the distance between their chosen locations.

I can't really move each of the ValueEventListeners to separate functions since they all have to fire in succession.

How do I make this code look good?

private void getUsersToDisplay() {
    // Get users to display from firebase
    Log.w(TAG, "User fetch initialized");

    DatabaseReference currentUserRef = mDatabase.getReference().child("users/" + mCurrentUser.getUid());

    // Get current user from firebase
    currentUserRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot currentUserSnapshot) {
            // Get the users chosen location for use in distance calculation
            final Double currentUserChosenLatitude =
                    Double.parseDouble(currentUserSnapshot.child("chosen_location/latlng/latitude").getValue().toString());
            final Double currentUserChosenLongitude =
                    Double.parseDouble(currentUserSnapshot.child("chosen_location/latlng/longitude").getValue().toString());

            DatabaseReference usersRef = mDatabase.getReference().child("users");

            // Get all users from firebase
            usersRef.addListenerForSingleValueEvent(new ValueEventListener() {
                @Override
                public void onDataChange(DataSnapshot allUsersSnapshot) {
                    // For all users in firebase
                    for (DataSnapshot userSnap : allUsersSnapshot.getChildren()) {
                        String userId = userSnap.getKey();

                        DatabaseReference userRef = mDatabase.getReference().child("users/" + userId);

                        // If the user isn't the current user
                        if (!userId.equals(mCurrentUser.getUid())) {
                            // Get the user's info from firebase
                            userRef.addListenerForSingleValueEvent(new ValueEventListener() {
                                @Override
                                public void onDataChange(DataSnapshot userSnapshot) {
                                    Double userChosenLatitude =
                                            Double.parseDouble(userSnapshot.child("chosen_location/latlng/latitude").getValue().toString());
                                    Double userChosenLongitude =
                                            Double.parseDouble(userSnapshot.child("chosen_location/latlng/longitude").getValue().toString());

                                    float[] results = new float[1];
                                    Location.distanceBetween(
                                            currentUserChosenLatitude,
                                            currentUserChosenLongitude,
                                            userChosenLatitude,
                                            userChosenLongitude,
                                            results);

                                    // If the user is within 10km of the current user, display it
                                    if (results[0] < 10000) {
                                        users.put(userSnapshot.getKey(), (String) userSnapshot.child("first_name").getValue());
                                        mUserAdapter.add((String) userSnapshot.child("first_name").getValue());
                                        Log.w(TAG, "User to display fetch complete");
                                    }
                                }

                                @Override
                                public void onCancelled(DatabaseError databaseError) {
                                    Log.w(TAG, "User to display fetch cancelled");
                                }
                            });
                        }
                    }
                    Log.w(TAG, "Users fetch completed");
                }

                @Override
                public void onCancelled(DatabaseError databaseError) {
                    Log.w(TAG, "Users fetch cancelled");
                }
            });
            Log.w(TAG, "Current user fetch complete");
        }

        @Override
        public void onCancelled(DatabaseError databaseError) {
            Log.w(TAG, "Current user fetch cancelled");
        }
    });
}
AL.
  • 36,815
  • 10
  • 142
  • 281
Dockson
  • 542
  • 2
  • 6
  • 16
  • You could create `ValueEventListener` object and pass that to `.addListenerForSingleValueEvent` instead of creating Anonymus callback object – Shreyash S Sarnayak Jan 03 '17 at 17:39
  • Yes but since they all have to fire in succession, the first ``ValueEventListener``will still contain all the other ones. I don't think it would make a huge difference. – Dockson Jan 03 '17 at 17:41

1 Answers1

1

I write my code like this so it easier to read:

onCreate() {
    dataRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            doSomething(dataSnapshot);
            ...
        }
        ...
    }
}

doSomething(DataSnapshot dataSnapshot) {
    ...
}

If I want a Firebase call to run after another Firebase call, I place it inside doSomething.

BUT, if that call doesn't have to run after each other (like "get current user" and "get all user" call from your sample code), I made it like this:

boolean firstCallDone = false;
boolean secondCallDone = false;

DataSnapshot firstDataSnapshot = null;
DataSnapshot secondDataSnapshot = null;

onCreate() {
    firstRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            firstCallDone = true;
            firstDataSnapshot = dataSnapshot;

            if (firsCallDone && secondCallDone)
                doSomething();
        }
        ...
    }
    secondRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            secondCallDone = true;
            secondDataSnapshot = dataSnapshot;

            if (firsCallDone && secondCallDone)
                doSomething();
        }
        ...
    }
}

doSomething() {
    // do something with firstDataSnapshot and secondDataSnapshot
    ...
}

Hope this help.

koceeng
  • 2,169
  • 3
  • 16
  • 37