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

Refactor simulateGrid method of DBFSAlgorithm to reduce complexity #818

Closed
wants to merge 10 commits into from

Conversation

danielfeismann
Copy link
Member

@danielfeismann danielfeismann commented May 29, 2024

Merge #813 and #823 before

Resolves #817

@danielfeismann danielfeismann added the code quality Code readability or efficiency is improved label May 29, 2024
@danielfeismann danielfeismann self-assigned this May 29, 2024
@danielfeismann danielfeismann added this to the Version 4.0 milestone May 29, 2024
@danielfeismann danielfeismann marked this pull request as ready for review May 29, 2024 11:01
@staudtMarius
Copy link
Member

@danielfeismann Would it be possible to also merge #823 before?

@danielfeismann
Copy link
Member Author

@danielfeismann Would it be possible to also merge #823 before?

Yes, we should do that.

Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. To be honest, I don't know if I like this more than the way it was before. Now, in order to understand the code, one has to "follow the functions" and move back-and-forth between the function definitions and the places that they are called, all in the same file.

The SonarQube metric of "Cognitive Complexity" seems to be flawed in itself, to say the least. The method in question is defining a pekko Behavior and is itself divided into sections of handling different types of messages. But even if a method is just that long, it doesn't have to be a bad thing to me.

One way to (arguably) more accurately represent the involved complexity is, in my opinion, using the concept of (for a lack of a better name) "DataCores" that encapsulate the data storage and processes involved with the data. It's what I used when modeling the new scheduler and EmAgent. This way, the code could maybe be simplified a bit. However, this suggestion should be taken with at least a grain of salt, as I might be a bit biased towards it.

@danielfeismann
Copy link
Member Author

danielfeismann commented Sep 13, 2024

I can understand your point of view and agree for some parts. I wanted to tackle the SonarQube Issue but on the other hand not to include too much effort. Thus splitting become an option to get "smaller" parts (which may be improved later). If we all agree to accept exceeding the metric in these cases, I'm fine with closing this one.

@danielfeismann danielfeismann deleted the df/#817-reduce-complexity-dbfs-simulateGrid branch October 31, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Code readability or efficiency is improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor simulateGrid method of DBFS to comply with sonar
3 participants