Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the cognitive complexity issues from the Sonar Cloud analysis #113

Closed
lmalenfant opened this issue Oct 6, 2023 · 5 comments
Closed
Assignees

Comments

@lmalenfant
Copy link
Member

Describe the bug
There are 120 issues on SonarCloud with high severity, most of these are cognitive complexity. This issue will address those particular code smells.

Application (MCCL, MATLAB or VTS code):
VTS

Additional context
https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&types=CODE_SMELL&id=VirtualPhotonics_VTS

@lmalenfant lmalenfant self-assigned this Oct 6, 2023
@dcuccia
Copy link
Contributor

dcuccia commented Oct 6, 2023

I would seriously hesitate to implement some of these, especially the ones concerning arrays. Time invested here could be spent on other more value-producing outcomes.

@lmalenfant
Copy link
Member Author

I would seriously hesitate to implement some of these, especially the ones concerning arrays. Time invested here could be spent on other more value-producing outcomes.

I'm not touching the arrays, only the cognitive complexity.

@dcuccia
Copy link
Contributor

dcuccia commented Oct 6, 2023

I'm not touching the arrays, only the cognitive complexity.

But even there, the worst offenders are about serialization (a subject that might be completely re-thought, per today's earlier conversation) and it's code that nobody every touches or needs to interact with, is all in one place, and has worked flawlessly for two decades. Wouldn't we rather spend time on higher-level concerns that benefit the end-user?

@lmalenfant
Copy link
Member Author

Making the code less complex and more maintainable will only aid in making changes like the ones we are discussing in the other issue. This is not something that we are definitely moving forward with, since @hayakawa16 works with the detectors on a weekly basis, any changes have to facilitate that work and not make it more difficult.

I created a draft pull request so @hayakawa16 can follow the process, if this is not a good partial step towards a reconfiguration, we don't have to continue.

@lmalenfant
Copy link
Member Author

Some of these issue will be addressed by #112. We will create a separate issue at a later time to fix the complexity in the areas of the code outside of detectors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants