phpmetrics / PhpMetrics

Beautiful and understandable static analysis tool for PHP
https://phpmetrics.github.io/website/
MIT License
2.48k stars 259 forks source link

Add a CCN value to every method in a class #481

Closed kudashevs closed 1 year ago

kudashevs commented 2 years ago

I've found your package very useful. I want to thank you for the great package.

During the usage, I realized that there is a thing which I really miss in this package. The possibility to see the Cyclomatic complexity of the most complex method in a class is useful, but I really wanted to see the complexity of every method in a class. So, my proposition is to add this possibility, which I think is quite useful. For now it looks like this.

image

niconoe- commented 2 years ago

Thank you very much for this feature, it's an interesting one to go deeper into the details!

On UI/UX side, I'm afraid this could be too verbose on projects with many classes. Do you think you could handle it via an accordion on the class line? A kind of triange-arrow-right could be added for each class-line and onclick on that arrow or the class name, the accordion would collapse (triangle-arrow rotates to be oriented to bottom) revealing the CCN for every methods of the current clicked class.

The default verbosity would still be the same so that current users won't get lost by too much data displayed, but at least the details by method would become available on demand thanks to your work.

kudashevs commented 2 years ago

Hello,

Here is a solution. It took a little bit longer because of table layout and the sorting functionality (that sorts metrics when you click on a header). It was impossible to add a new table row (because it automatically becomes sortable, and it doesn't make any sense). So, my solution was to add a div element that has an absolute positioning. This element is hidden by default and you can open it by clicking on the class name.

The only problem that I had with this solution is that the new div element wasn't positioned relatively to the table with metrics. To overcome this difficulty I wrote a short code that collects the width values of metrics columns and applies them to the "columns" of the new div element. It does the recalculation on a window resize too. I've attached a picture of how it works.

image

niconoe- commented 2 years ago

Thank you very much! This looks nice to me!

@Halleck45 what do you think about?

Mte90 commented 1 year ago

Maybe for next 3.0.0 @niconoe-?

kudashevs commented 1 year ago

Hello @Halleck45

Could you please provide any feedback about this PR?

This functionality was requested long before in #444 and I find it very useful. Do you like the whole idea? Should I refine or improve something in this PR?

niconoe- commented 1 year ago

I think that rather having a table component, we should migrate to something more flexible to allow sub-tables. I'll try to take some time to search for reliable components about it.

niconoe- commented 1 year ago

Sorry for the waiting about the PR. I had to take a long break recently.

@kudashevs I reworked the incoming v3 with your work in an adapted way in order to manage both UX/UI and not to interfear too much with logic.

I go further than your proposal by managing to display the accessors even if they are ignored from the cyclomatic complexity calculations, with a flag to explain this to the end-user. I also managed to open the rows via an accordion style without having too much JS changed, and keeping the possibilty to sort rows by their column values.

Here is a sample of how it's displayed now: selection

I think this way, it's better, and it opens the way to having other metrics on the methods in the future.

It will be part of the v3 and will probably be released in the next RC.

Feel free to suggest or remark stuff about it 😉

kudashevs commented 1 year ago

That looks good. Hope to see this functionality soon in the V3 RC.