Hello Sebastian, If I do remember correctly `StreamProcessor::streamByUid <Map with Streams by UID>` was created to speed-up stream search (to avoid iterating all KRooms) The main source-of-truth should be KRoom
Duplication is bad thing - no doubt but I thought extra-iterating is worst :) I tried to make both consistent ... are you sure they are out of sync? I see no value in IKStreamManager due to: 1) it will be implemented only once 2) I expect no inter-module troubles here Maybe we can get rid of KRoom map due to stream UID should be unique and streamByUid can be retrieved in constant time Does it make sense? On Mon, 27 Apr 2020 at 10:25, seba.wag...@gmail.com <seba.wag...@gmail.com> wrote: > > I also can see that those lists have slightly different meaning > > 1. StreamProcessor::streamByUid <Map with Streams by UID> > => Will be populated when the stream start > 2. KRoom::streams <Map with Streams by UID> > => Will be populated when KStream is created > > Still that would be no problem to replicate this functionality in 1 place. > > Cheers > Seb > > Sebastian Wagner > https://www.linkedin.com/in/sebastianwagner/ > > > On Mon, 27 Apr 2020 at 13:43, seba.wag...@gmail.com <seba.wag...@gmail.com> > wrote: >> >> This should be actually a pretty simple refactor. And much easier than >> trying to replicate StreamHandler events in to the KRoom. >> >> As well as having a defined API in the IKStreamHandler. >> >> Cheers >> Seb >> >> >> Sebastian Wagner >> https://www.linkedin.com/in/sebastianwagner/ >> >> >> On Mon, 27 Apr 2020 at 10:33, seba.wag...@gmail.com <seba.wag...@gmail.com> >> wrote: >>> >>> Hi, >>> >>> currently we have 2 places that hold a reference to the KStream objects: >>> >>> StreamProcessor::streamByUid <Map with Streams by UID> >>> KRoom::streams <Map with Streams by UID> >>> >>> Removing it from one place may or may not result removing it in the other >>> place. >>> This can lead to memory leaks. As well as those lists can get out of sync. >>> And you are also duplicating code in two places. >>> >>> Could we explore if we can add a similar construct to the KStream that >>> exists for Client objects using an abstract IClientManager and an >>> implementation ClientManager ? >>> >>> Similar adding a Class KStreamManager and an Interface IKStreamManager. And >>> reference this single object in both places >>> IKStreamManager can define methods to get either Streams by RoomId or by >>> UID or by SID. Or other abstract methods. >>> >>> This could help a lot with making sure when adding or removing a KStream, >>> there is a single place that holds the reference. >>> >>> What do you think? >>> >>> Thanks, >>> Seb >>> >>> Sebastian Wagner >>> https://www.linkedin.com/in/sebastianwagner/ -- Best regards, Maxim
