code-complexity icon indicating copy to clipboard operation
code-complexity copied to clipboard

Published 20 hours ago • verified publisher icon simonrenoult

Added support for choosing complexity strategies

Open scottamplitude opened this issue 4 years ago • 7 comments

Created built-in support for SLOC, Cyclomatic & Halstead complexity support with new CLI param complexity-strategy.

scottamplitude avatar Aug 24 '21 05:08 scottamplitude

Oh wow. Thanks for your contribution. It looks promising, I'll look into it :+1:

simonrenoult avatar Aug 24 '21 22:08 simonrenoult

How's this look to you @simonrenoult ?

scottamplitude avatar Sep 02 '21 23:09 scottamplitude

Hi @scottamplitude,

I love the idea. Two things we should consider:

  1. We loose the ability to evaluate other languages than JS/TS. While imperfect, node-sloc allowed code-complexity to support lots of languages. IMO, the Sloc strategy should keep using node-sloc while Halstead/Cyclomatic strategies should fail if the files analyzed are not JS/TS. This way we get the best of both worlds. :partying_face:

  2. Writing our own Halstead/Cyclomatic analyzers implies that we will maintain them. I would rather use an npm package for that but it doesn't seem like it exists off the shelf :disappointed: However, I consider the added-value of theses strategies too good to miss so I'll happily welcome them. I'll reach out the ts-complex people to see if we can sort something out.

Also, it feels like the PR lacks proper tests. We could unit test the strategies, wdyt? :thinking:

simonrenoult avatar Sep 06 '21 10:09 simonrenoult

All great points. I hadn't realized the intent was to support non-JS/TS based languages. Totally understandable that this would be the wrong route then.

One note RE: # 2 I initially tried implementing the new complexity strategies with https://github.com/escomplex/escomplex since you'd mentioned it in https://github.com/simonrenoult/code-complexity/issues/5, but was unable to get it working. I could spend some more time looking into it again to see if I missed something. If you have any other libraries in mind that you want to leverage, I could look into those as well.

+1 to tests for each strategy.

scottamplitude avatar Sep 08 '21 01:09 scottamplitude

@simonrenoult hope all is well! Are you evaluating some third-party open source solutions for supporting these new strategies? If you've found a library or a couple of libraries you'd like to use I'd be happy to work with you to implement them in your codebase. Just let me know what you picked and I'll update this PR or create a new one.

scottamplitude avatar Sep 10 '21 17:09 scottamplitude

Hi,

All good, thanks :+1: , I hope you are good as well. I think we can forget externalizing the strategies for now, it's not the priority. My others remarks remain (using node-sloc for the sloc strategy + checking for js/ts when using halstead/cyclocmatic strategies + unit tests on halstead and cyclomatic) if that's good for you. Wdyt?

simonrenoult avatar Sep 10 '21 23:09 simonrenoult

I took the liberty to fork your PR in order to implement my own feedbacks: #30

It's quite close to being done. I just need to add a few unit tests on the complexity calculation and a way to choose one of the halstead metric from the CLI.

Awesome work btw, it really helped me grasp how to implement cyclomatic and halstead strategies :clap:

simonrenoult avatar Sep 18 '21 01:09 simonrenoult

I just merged an alternative version using escomplex. Available in 4.3 :)

simonrenoult avatar Nov 11 '22 11:11 simonrenoult