cqfn / jpeek

Hosted and command-line calculator of cohesion metrics for Java code
https://i.jpeek.org
MIT License
209 stars 81 forks source link

added ccmCalculus especially to find ncc value for ccm metric. The lo… #548

Open starkda opened 8 months ago

starkda commented 8 months ago

…gic that finds connected components not implemented yet. one of pull requests that resolve https://github.com/cqfn/jpeek/issues/522

In this pr I've added separate calculus that would be used to find CCM metrics. It would traverse over existing XML skeleton, and add . NCC it is not calculated yet, it is always 1. In further prs I would add logic for it and sligthly modify CCM.xsl to capture ncc from skeleton.

starkda commented 8 months ago

@yegor256 please review

yegor256 commented 8 months ago

@pnatashap can you please take a look?

pnatashap commented 8 months ago

@starkda as implementation is not completed, all things that are not completed should be in @todo puzzle in the code, not in PR. Pull request will be closed and all future plans from it will not be accessible for anybody, but their must be.

starkda commented 8 months ago

@pnatashap please review

pnatashap commented 8 months ago

@starkda looks like some loop is possible, test for macos with jdk 11 failed due time-out (6 hours)

pnatashap commented 8 months ago

@starkda It would be better if you create all required tests with manually calculated metrics (ccm and all parts inside) to understands what we want to achieve. (should be with one connectivity component inside, with several of them, with and without constructor inside). This will really add some value for the next steps.
And I do not see any huge problem to calculate it inside of xsl: all information about methods and attribute usage, so using call-template with parameters looks like possible to divide it into the groups. Even if we can not calculate something in xsl, we can calculate it and pass as a parameter to XSL, because reading all this xml document transformation is really very difficult.

starkda commented 8 months ago

@starkda looks like some loop is possible, test for macos with jdk 11 failed due time-out (6 hours)

@pnatashap I guess it is not problem from the changes i made. I effectively changed nothing, but now it works

starkda commented 8 months ago

@starkda It would be better if you create all required tests with manually calculated metrics (ccm and all parts inside) to understands what we want to achieve. (should be with one connectivity component inside, with several of them, with and without constructor inside). This will really add some value for the next steps. And I do not see any huge problem to calculate it inside of xsl: all information about methods and attribute usage, so using call-template with parameters looks like possible to divide it into the groups. Even if we can not calculate something in xsl, we can calculate it and pass as a parameter to XSL, because reading all this xml document transformation is really very difficult.

@pnatashap,

  1. You're right. I'll add more test in further pull request(I've added puzzle in code for more tests)
  2. That's what i did: we cannot calculate number of connected components(ncc) in xsl so I passed it as a parameter. Yet it looks terrifying I guess that's the best option, I wish I could delegate some steps to xsl, but the problem in very last step: running dfs.
pnatashap commented 8 months ago

@starkda you have already one example for the test in issue, it is very easy to add a test for it. all other code does not bring anything usefull in the project, just brake existing even more

pnatashap commented 8 months ago

@starkda I've added a tests and find an issue with nc calculation. It is definitely can be solved in xsl (and the issue with build freezing is also solved)

starkda commented 8 months ago

@pnatashap could you please elaborate how it can be made in xsl. Any approach requires to change state while xsl variables can not be changed after initialization

pnatashap commented 8 months ago

@starkda for fix nc calculation you just need to add one more condition in if and that it. variables in xsl can not be changed, but you can call template with any value that you need. I still agree with this comment https://github.com/cqfn/jpeek/issues/523#issuecomment-847514690

starkda commented 8 months ago

@yegor256 @pnatashap there is a problem with https://github.com/starkda/jpeek/blob/6bbc1ee751cdea196c6dfa11d2c1245d5d0cf8e5/src/test/java/org/jpeek/web/ReportsTest.java#L61. I get IO java.util.concurrent.ExecutionException: java.lang.OutOfMemoryError: Java heap space full stacktrace: https://pastebin.com/L5GdfHy0 The problem is definitely in changes of CCM.xsl i made. I tried to optimize dfs a little bit, but it did not helped. Do you have any ideas how to optimize memory usage and pass the test?

starkda commented 7 months ago

@pnatashap what about returning to my initial approach and use java instead of xsl to calculate ncc? I suppose the main problem with heap overflow can not be solved anyway. Such solution eats a lot of memory and may not be acceptable for more or less big repos

pnatashap commented 7 months ago

@starkda you can try to calculate it, not just add a comment, that it should be calculated here, as we have already a task to calculate it properly

starkda commented 7 months ago

@pnatashap please review. hope it more readeable now. To find components i used union find instead of dfs

starkda commented 7 months ago

@pnatashap please review

starkda commented 7 months ago

@pnatashap please review I've put Union-Find into separate class. I've also created separate package for it, as i did not find suitable place for it. Graph package is not a good place for it i think, as it is related to xml.

starkda commented 7 months ago

@pnatashap I re-implemented CcmCalculus using XmlGraph. I also added several xsl files to add meta information without in-code xml modification. XmlGraph implementation has some bugs, so it does not always give correct ncc.