With typed properties in PHP 7.4, there comes a natural tendency for using properties as public. The type check is already there, right? It's a dangerous path that opens the door to static code with public properties everywhere, that is asking for a change in any place.
But is that ok to show privates to the public?
Last week you probably spotted RFC: Constructor Promotion proposed by my favorite mentor Nikita Popov. This RFC counters public property by default approach and reduces the redundant code at the same time.
Another counter point for public properties or methods nailed Stefan Priebsch:
That meme had just 1 public method. But what about real projects?
To get real numbers from a real project, I run phploc to measure size of Rector's code.
phploc src packages rules --exclude tests
These are the results:
Size
Non-Comment Lines of Code (NCLOC) 99033 (85.46%)
Structure
Methods 5185
Visibility
Public Methods 3516 (67.81%)
Let's put 100 children in one square:
This is 3 500 children:
There is ~480 Rector rules, each has 3 public methods required by interface contract. That's 1 500 public contract methods. Even if we remove those, 3 500 - 1 500, we still have over 2 000 public methods:
Good luck with getting all your 2 000 children to university... or bed ... or make them breakfast for them... for one day. No pressure, right?
We want to reduce the number of public methods to a minimum. How?
We can go for a method by method refactoring with careful analysis if the method should be public or private and how we can change it to private. It takes time, patience, attention, human performance and doesn't scale on massive projects. That's way too expensive.
❌
What simpler way can we apply today on an any-sized project? What low hanging fruit can we focus on?
What is fake public method, property, or constant?
<?php
declare(strict_types=1);
final class SomeController
{
/**
* @Route(path="/")
*/
public function homepage()
{
return $this->prepareData();
}
public function prepareData(): array
{
return ['status' => 'quarantine'];
}
}
Can you see it?
+public function prepareData(): array
-private function prepareData(): array
✅
Same applies for constants and properties, that is used only locally in the class they're defined in:
<?php
declare(strict_types=1);
final class AirCleaner
{
public const NAME = 'cleaner';
public function getName()
{
return self::NAME;
}
}
-public const NAME = 'cleaner';
+private const NAME = 'cleaner';
✅
The most common false publics are constants because they're last that got visibility in PHP 7.1.
"Nice intellectual exercise, Tom, but nothing more," you may think, and close this post and go back to your quarantine work.
But remember our children:
A public method is like a child: Once you've written it,
you are going to maintain it for the rest of its life.
What happens if we don't care about it? Well, public
element is designed to be used somewhere else.
Same way static
method is a method to be used everywhere (and then slowly kill you/).
It's easy to spot now because we focus on ten lines of code, we knew that's a controller method only, and that's a clear miss-use. But in the real world, we don't have so much time to think about 10 lines of code.
10 lines of code = 10 issues.
— I Am Developer (@iamdevloper) November 5, 2013
500 lines of code = "looks fine."
Code reviews.
Limited attention span is one of the reasons. We had plenty of such potential legacy back doors in Rector:
Another reason is that the method is completely unused, but PHPStorm won't tell you because it is public
. When we turned this method into private
, we saw it's unused, and we got rid of it:
The effect of privatization is surprising. Take this case:
How can the privatization of methods lead to more code? Easily. The method was public
, but never used. After Rector run it was private
, but never used and then removed... and that could work.
Actually, that was not a feature, it was a bug. This method should have been used many months ago. So we used it in the right place, the feature was complete, and potential future bug.
Do you want to know all the possible code changes?
See pull-requestWhat now? Go to your code and check method one by one clicking on them in PHPStorm to Find Usages, if they're used somewhere else or false public and should be private. It will make your code much more robust and senior...
Just kidding. Let Rector handle it with PRIVATIZATION
set:
use Rector\Set\ValueObject\SetList;
use Rector\Config\RectorConfig;
return function (RectorConfig $rectorConfig): void {
$rectorConfig->import(SetList::PRIVATIZATION);
};
vendor/bin/rector process src
It has now 4 rules:
Do you have an idea for another privatization rule? Let us know on GitHub.
Stay lazy and alive!
Happy coding!
Do you learn from my contents or use open-souce packages like Rector every day?
Consider supporting it on GitHub Sponsors.
I'd really appreciate it!