Remove onGeoQueryReady

[vanniktech]

I don't quite understand why there is a need for this event. Also, there's quite a lot of logic involved.

Is there any use case for this? Are many people using it? If not I'd vote to remove it to have a leaner, smaller API footprint.

[samtstern]

@vanniktech I believe the use case for this is similar to onDataChanged in the Realtime Database SDK. Basically this lets you know that you have received all of the onKey{Blah} events that you will get in the first batch.

This helps if you are doing UI operations when keys appear/disappear that would benefit from batching. For example maybe you're adding all of the items to a RecyclerView and only want to refresh the view after all the locations are loaded. Or maybe you have a "Loading" indicator you want to dismiss after the GeoQuery resolves for the first time.

[vanniktech]

Interesting use case but would not you also want to know about the 2nd time when the query was executed and done?

Maybe the GeoQuery class would benefit from becoming immutable so if you want a new Query you'll need to use the queryAt function and get a new GeoQuery instance back.

[samtstern]

Interesting use case but would not you also want to know about the 2nd time when the query was executed and done?

Yes you're probably right, and this is what happens with onDataChanged. So maybe this would be more useful if we always fired an onGeoQueryReady event after each batch of child events. In which case we'd probably want to rename the callback.

[vanniktech]

I've given this some more thought and I don't see the value of really having this onGeoQueryReady function. It would be interesting to see how many people are using this and how.

[ahaverty]

We rely on onGeoQueryReady for the use cases @samstern laid out with our recycler views and now also geofire JavaScript in our backend. It's been working perfectly, although we're dealing with static locations and a center location that doesn't move, so we're essentially using it to perform a "single" listener.

[vanniktech]

So when you get the event you'll dismiss the loading indicator in the recyclerview?

[ahaverty]

Exactly, and in our case we also kill the listener since our use case doesn't require any update events.

[vanniktech]

Interesting this is yet another behavior which is different from the current use case without the onGeoQueryReady and delta updates (#88).

I think having one listener that's capable of everything is not really feasible.

[puf]

The initial data load (in both regular Firebase and GeoFire) is usually much bigger than subsequent delta data updates. For this reason many apps want to gather the initial screen updates in a batch, while they'll pass subsequent updates to the screen one-by-one. Removing onGeoQueryReady would make that harder.

[vanniktech]

Alright. I'll close this then since there's an obvious need for this. I'll try to think how we can do the listeners better nonetheless though and possible don't require doing all of the heavy work in case you don't need it. onGeoQueryReady in this case.