-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
@danielfeismann Would it be possible to also merge #823 before? |
Yes, we should do that. |
…ulateGrid # Conflicts: # CHANGELOG.md
…ulateGrid # Conflicts: # src/main/scala/edu/ie3/simona/agent/grid/DBFSAlgorithm.scala
…ulateGrid # Conflicts: # CHANGELOG.md
…ulateGrid # Conflicts: # CHANGELOG.md
There was a problem hiding this 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.
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. |
Merge #813 and #823 before
Resolves #817