[GitHub] [openmeetings] sebawagner commented on pull request #135: OPENMEETINGS-2585 IKurentoHandler class and add methods to IStreamHandler

[GitBox <gi...@apache.org>]

sebawagner commented on pull request #135:
URL: https://github.com/apache/openmeetings/pull/135#issuecomment-790439102


   > I'm -1 for the changes like it is here :(
   > 
   > we definitely not need these huge interfaces at this stage
   > also we don't need to make all methods public as well as some are not static anymore
   > 
   > I would suggest to make `necessary changes` only
   
   You can probably refactor some of the methods further to get the size of the interface smaller. 
   
   The problem is: Those classes have never been designed in a way so you can easily introduce a new behaviour. Except all new behaviour is in the OpenMeetings-core module.
   
   There is a lot of functionality in the StreamProcessor and KurentoHandler that have nothing to do with Streaming or Kurento:
   - All those "static" methods. Why are they in those classes? They belong into a Util class.
   - There are a lot of methods to "checkRights" in the StreamProcessor, what for? Those methods only check on the "Client" nothing to do with Streaming or StreamProcessor
   - A lot of the methods are just "facade", for example streamProcessor::isSharing => Calls kHandler.getRoom() => Why not directly calls kHandler.getRoom() ? I has nothing to do with Streaming.
   - StreamProcessor::startConvertion => Why is that in the StreamProcessor ? It starts a new thread to process a recording/. It has nothing to do on the stream. Doesn't require anything from it. There is just no reason why its in this class
   
   And the list could be going on for a lot longer.
   
   I think it would be great to refactor those methods out of those classes. And then those interfaces would become a lot smaller and better (and also a lot easier to use!).
   
   And then it would be a lot easier to actually do the different Kurento and Stream handling! Cause you don't need to worry about all those other internal OpenMeetings functionality!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org