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

Suggestion to change the scoring #97

Closed
matthiasnoback opened this issue Sep 21, 2017 · 7 comments
Closed

Suggestion to change the scoring #97

matthiasnoback opened this issue Sep 21, 2017 · 7 comments

Comments

@matthiasnoback
Copy link
Contributor

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?

@bmitch
Copy link
Owner

bmitch commented Sep 21, 2017

Thanks @matthiasnoback !

Yes I agree 100%.

I think it would be ideal to display the results in 4 sections:

  • High complexity and changed often.
  • High complexity and not changed often.
  • Low complexity and changed often.
  • Low complexity and not changed often.

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?

@Lucky-Loek
Copy link
Contributor

Lucky-Loek commented Sep 27, 2017

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

@bmitch
Copy link
Owner

bmitch commented Sep 28, 2017

@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".

@matthiasnoback
Copy link
Contributor Author

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.

@bmitch
Copy link
Owner

bmitch commented Sep 28, 2017

@matthiasnoback That would be great! Looking forward to what you put together.

@matthiasnoback
Copy link
Contributor Author

Oops, sorry...

@bmitch
Copy link
Owner

bmitch commented Oct 6, 2017

Resolved by #122

@bmitch bmitch closed this as completed Oct 6, 2017
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

No branches or pull requests

3 participants