sebawagner commented on pull request #136: URL: https://github.com/apache/openmeetings/pull/136#issuecomment-791830282
> Hello @sebawagner, > > it seems my English is not understandable > > I see no need in these changes at this stage > > additional constructor params are pure evil :( leading to the code like this: https://sonarcloud.io/project/issues?id=apache_openmeetings&resolved=false&rules=java%3AS107&types=CODE_SMELL > > I would once again suggest you to create some sort of cluster ready code (in OM or separate repo) before refactoring OM code .... Arguing that with the number of parameters initialising dependencies of a class via constructor is a "code-smell" it sounds far fetched. That is basically why you have @Autowired and other annotations, so that the constructor doesn't get too big. But the problem with those classes are that they are not beans. They are a mix of "behaviour" and "storage" which is the nature of the problem why you need some strange constructs in order to load dependencies into them. Behaviour should be in Controllers. Holding information should be DTOs. Mixing them leads to issues. Which is one of the reasons why it would be good to refactor this part of the code. ---------------------------------------------------------------- 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: [email protected]
