search

deepstreamIO / deepstream.io-client-java

The Java/Android Client for deepstream.io
Other
35 stars 37 forks source link

Add whenReady to the client API. #108

Closed fhriley closed 5 years ago

fhriley commented 7 years ago

Make Record.whenReady() public so it can be used by clients. Add public List.whenReady() so clients can be notified when Lists are ready.

AlexBHarley commented 7 years ago

@fhriley thanks for the contribution, am happy with the whenReady addition to the List class.

One thing though, we haven't made Record.whenReady public because the call to RecordHandler.getRecord is synchronous and actually uses Record.whenReady internally.

I think for consistency and simplicity it would be better to make RecordHandler.getList synchronous as well and just use whenReady internally.

What do you think?

fhriley commented 7 years ago

RecordHandler.getRecord being synchronous isn't a problem, is it? Record.whenReady checks the isReady flag first so it will immediately return in that case. Personally, I'd prefer not to make an asynchronous operation synchronous. If anything RecordHandler.getRecord should be made asynchronous if it can be (not a task for this PR, though, IMO).

fhriley commented 7 years ago

I looked at the code some more. Isn't RecordHandler.getList also synchronous? When creating a new list, it calls RecordHandler.getRecord to get the record, and as you've said, that's synchronous.

And somewhat related, it looks like the code in RecordHandler that deals with records, lists, and listeners is not thread safe. There is some double checked locking being used, but it's not safe. A HashMap is not thread safe so doing a get the same time as a put is bad. Should I file an issue and fix this?