-
Notifications
You must be signed in to change notification settings - Fork 56
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
Suggestion to change the scoring #97
Comments
Thanks @matthiasnoback ! Yes I agree 100%. I think it would be ideal to display the results in 4 sections:
But I am not sure what the best way to display this would be. In that article and other tools I've seen for ruby they will produce an image. That is nice but I'd like to have something that is visible when you run this during your CI build. Do you have any suggestions? |
Maybe we could implement a flag or option so a user can specify what "table" he wants to see? By default it would be high complexity and changed often, but by using flags or options, the corresponding tables would also be printed? For example: this would print a table with all candidates found for lots of changes, but low complexity. $ php churn run src -low-complexity-high-changes
# Or
$php churn run src --show=low-complex-high-changes While this would print the default (high complexity, lots of changes) and the above: $ php churn run src -default -low-complexity-high-changes |
@loekiedepo I think that something like that would be the end goal for sure. Maybe as a first step I should display the 4 tables first. Then can add options to hide/show the ones you want. I'm kind of thinking out loud here, trying to decide how to split up the four tables. I suppose it would need to determine what is considered "high" or "low" for both changes and complexity and relative to all the other files that were scanned. Guess we could just take the highest score and divide that in half. Anything above is considered "high" anything below is considered "low". |
I've been thinking about the calculations too and I think we could still do this with one score. I tried calculating the distance from the lower left corner, but that didn't help much. Maybe we could calculate the distance from the top right corner, then cut off at a configurable number. This is high on my wishlist, so I'll take some time somewhere during the following weeks to fiddle with it. |
@matthiasnoback That would be great! Looking forward to what you put together. |
Oops, sorry... |
Resolved by #122 |
Thanks for a nice tool!
The way I understand it (and the way it's described in this article: https://www.sandimetz.com/blog/2017/9/13/breaking-up-the-behemoth), we should consider to refactor classes that are high on high on both complexity and times changed. Otherwise, we would start refactoring complex classes that change almost never (which is a waste of effort) or we would start refactoring simple classes that change often (which also makes no sense, since it's very easy to make the change, so it doesn't cost us much).
What do you think?
The text was updated successfully, but these errors were encountered: