bmitch / churn-php Goto Github PK

For example, when providing the path src right now it displays the files like:

+------------------------------------------------------------------------------------------------+---------------+------------+-------+
| File                                                                                           | Times Changed | Complexity | Score |
+------------------------------------------------------------------------------------------------+---------------+------------+-------+
| /home/bmitch/Code/ChurnPhp/src/Assessors/CyclomaticComplexity/CyclomaticComplexityAssessor.php | 4             | 4          | 8     |
| /home/bmitch/Code/ChurnPhp/src/Assessors/GitCommitCount/GitCommitCountAssessor.php             | 3             | 4          | 7     |
| /home/bmitch/Code/ChurnPhp/src/Commands/ChurnCommand.php                                       | 3             | 2          | 5     |
| /home/bmitch/Code/ChurnPhp/src/Managers/FileManager.php                                        | 1             | 3          | 4     |
| /home/bmitch/Code/ChurnPhp/src/Results/ResultsGenerator.php                                    | 1             | 2          | 3     |
| /home/bmitch/Code/ChurnPhp/src/Services/CommandService.php                                     | 2             | 1          | 3     |
| /home/bmitch/Code/ChurnPhp/src/Results/ResultCollection.php                                    | 1             | 1          | 2     |
| /home/bmitch/Code/ChurnPhp/src/Results/Result.php                                              | 1             | 1          | 2     |
+------------------------------------------------------------------------------------------------+---------------+------------+-------+

Would be nice if it was like this instead.

+-----------------------------------------------------------------+---------------+------------+-------+
| File                                                            | Times Changed | Complexity | Score |
+-----------------------------------------------------------------+---------------+------------+-------+
| Assessors/CyclomaticComplexity/CyclomaticComplexityAssessor.php | 4             | 4          | 8     |
| Assessors/GitCommitCount/GitCommitCountAssessor.php             | 3             | 4          | 7     |
| Commands/ChurnCommand.php                                       | 3             | 2          | 5     |
| Managers/FileManager.php                                        | 1             | 3          | 4     |
| Results/ResultsGenerator.php                                    | 1             | 2          | 3     |
| Services/CommandService.php                                     | 2             | 1          | 3     |
| Results/ResultCollection.php                                    | 1             | 1          | 2     |
| Results/Result.php                                              | 1             | 1          | 2     |
+-----------------------------------------------------------------+---------------+------------+-------+

Because of this line:

https://github.com/bmitch/churn-php/blob/master/src/Factories/ProcessFactory.php#L35

Is there another command that will do the same thing that works in Windows and Unix?

Could show:

• Files analyzed.
• Time it took to run.
• ???

I think if you try to make a regex pattern like *.php to ignore files it won't give you the expected results.

I created this test:

        $fileManager = new FileManager(['php', 'inc'], ['Assets2/*.php']);
        $this->assertCount(1, $fileManager->getPhpFiles([__DIR__ . '/../Assets2']));

Which I would expect to pass but it fails.

• Result #27
• ResultCollection #30
• ResultsGenerator #34
• FileManager #31
• ChurnCommand (hmm not sure if it is possible right now, going to skip)

Stems from #40

I am envisioning something like this:

vendor/bin/churn-php src

+--------------+---------------+---------------+---------------+
| File         | Times Changed | CycComplexity | Score         |
+--------------+---------------+---------------+---------------+
| src/Foo.php  | 10            | 5             | 50            |
| src/Bar.php  | 5             | 7             | 35            |
| src/Baz.php  | 2             | 1             | 2             |
+--------------+---------------+---------------+---------------+

• Need to determine how to calculate score. Is it simply number of times changed X complexity?
• Use symfony console. http://symfony.com/doc/current/components/console.html
• Use symfony console table helper to display table. https://symfony.com/doc/current/components/console/helpers/table.html
• Store results of each file in an object, then all objects in a collection. Make it easy to sort.

See #78 (comment)

This new feature allows you to ignore files using a regex.

The readme.md should be updated to reflect that.

This can be a value added to the yml config. For example, if you have a large git commit history you may only want to limit the changes within the last year:

        $process = new Process(
            'git -C ' . getcwd() . " log --since=\"1 year ago\" --name-only --pretty=format: " . $file->getFullPath(). " | sort | uniq -c | sort -nr"
        );

For that for this feature I'd also want the user to be able to provide the paths via the churn.yml file if they like.

Possibly like:

folders:
 - src
 - tests

Stems from #40

A sniff to disallow the use of instanceof function would be good to have.

The use of it is a code smell and represents a flaw in the project Software Architecture.

I love this package that helps us to write more SOLID and Clean Code.

See the original comment here:

#104 (comment)

Stems from #40

Cyc Complex * changes?

Should one of those scores weigh more than the other?

Here:

    /**
     * Config constructor.
     * @param array $rawData Raw config data.
     */
    public function __construct(array $rawData = [])
    {
        $this->filesToShow = $rawData['filesToShow'] ?? 10;
        $this->parallelJobs = $rawData['parallelJobs'] ?? 10;
        $this->commitsSince = $rawData['commitsSince'] ?? '10 years ago';
        $this->filesToIgnore = $rawData['filesToIgnore'] ?? [];
    }

Though git is the most popular, mercurial is heavily used as well and it would be interesting to put this to use.

Stems from #40

• Make process of getting all the *.php files from one object.
• Make getting results of all the files from one object and should return an object.
• Make display of results one object.

This is due to the command used in the process factory.

Perhaps there's another way of doing the same command that will work in windows + unix platforms.

Might look better with a screen shot that shows the colours.

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?

• #38 - Run in parallel Done
• #40 - Config file (yml) Saving for later and can be backwards compatible new feautre.
• #41 - Add some sort of header above table Done
• #42 - Add footer with misc data under table? Saving for later and can be backwards compatible new feautre.

• Provide it the path to a file.
• Run the git command to see how many commits this file has.
• Return that value.

With a large number of files the results take a while to come back. Need some way to run the files in parallel.

• http://symfony.com/doc/current/components/process.html
• http://blog.servergrove.com/2014/04/16/symfony2-components-overview-process/

If we used something like the above then we'd need to probably create another command that simple takes the file path and returns the results.

Using set methods in a class is not advised, because:

• if the class is instantiated and we forget to call the set methods it may work or not and cause side effects. Plus we may need to call them in the correct order.
• if you need to inject Collaborators they must be strictly injected via constructor and any other values needed by the class should be passed as arguments to the method being called, preferable using Immutable Value Objects(the only exception to not inject an object via constructor).
• objects are passed by reference in Php, therefore when the object is passed around any call to the set method will affect all places where the object is being used. This can cause undesired bugs, that sometimes are hard to track.

I can continue with more cases why we should not use set methods, but I think the above 3 ones are strong enough to justify the sniff.

Thanks for your excellent work.

I'm open to reviews of the readme if anyone has any suggestions.

• Any information missing or instructions missing.
• Better layout/design/look.
• Anything else you'd like to see in the readme.

Still not entirely happy with the screenshot in the readme.md

• It has a command prompt at the top and the bottom. Could remove both, neither are really needed.
• It contains a really long file name (the CyclomaticComplexityAssessor.php) in the results which forces the table to be very wide. If that file was ignored the table would be smaller in width which I think would improve the screenshot.

In order to get the git commits we'll need to know the root folder so that we can run the git command.

We could also store other values in here:

• How many commits to go back (with a default value if not provided).
• How many of the top files to show (with a default)?
• Score modifier? (maybe save for later) #55
• Number of parallel jobs to run
• Threshold (only show results where score > x) (will save for later) #56
• Path of files (save for later) #57
• files to ignore (exact path and name now, regex later?) #58

Also need to make sure it works without a yml file present or some options not set.