Glimesh / janus-ftl-orchestrator

A service used to manage multiple instances of janus-ftl-plugin for use at scale.
GNU Affero General Public License v3.0
15 stars 3 forks source link

♻️ Refactor subscription getters #12

Closed danstiner closed 3 years ago

danstiner commented 3 years ago

No functional impact. I tend to learn by refactoring so no worries if this change isn't wanted.

Tried to follow what I understand is convention to map's find() for lookups, though it still seems awkward to compare with end(), https://stackoverflow.com/a/33239463

Simplified getter by returning a copy of the subscriber set directly instead of building a new vector by hand (experimented with other methods but a copy is needed afaict because the lock is only for this method, after it returns the set could end up changing).

Also taking in key parameters as const reference for clarity and skipping an unneeded copy. Kept them as shared_ptr for now because the map keys themselves are shared_ptrs.

haydenmc commented 3 years ago

Per offline discussion, we concluded that returning copies of ChannelSubscription was preferred instead of extending the lifetime of the subscriptions by returning shared_ptr.