Add Multiple Notifiers Support (Range and Monitor) #392
Conversation
| @@ -109,9 +109,9 @@ | |||
| protected static BeaconManager client = null; | |||
| private final ConcurrentMap<BeaconConsumer, ConsumerInfo> consumers = new ConcurrentHashMap<BeaconConsumer,ConsumerInfo>(); | |||
| private Messenger serviceMessenger = null; | |||
| protected RangeNotifier rangeNotifier = null; | |||
| protected final List<RangeNotifier> rangeNotifiers = new CopyOnWriteArrayList<>(); | |||
There was a problem hiding this comment.
I think it would be good if these collections were based on Set not List. Because if folks repeatedly call addXxxNotifier() with the same object, it could cause trouble. Making this change could protect against this. Perhaps use ConcurrentHashMap.newKeySet()?
There was a problem hiding this comment.
I think to use an CopyOnWriteArraySet. What do you think?
As consequences, the getMonitorNotifier() and the getRangeNotifier() method are irrelevants. Do you want to keep them or can I remove them ?
(I think these methods are only useful internally to the library. They are not in usage any more inside the library)
There was a problem hiding this comment.
We do need to keep getMonitorNotifier() and getRangeNotifier() so that this is not a breaking API change. I think a CopyOnWriteArraySet is probably fine.
|
@OPscT, do you need the Sorry to make you spin your wheels on this -- I know this is not your original PR. I appreciate your help. |
|
The update has been done |
Re-do the job done by @Maxeee09 in this pull request with a correct code style