Encapsulate processed data #749
Conversation
Codecov Report
@@ Coverage Diff @@
## master #749 +/- ##
============================================
+ Coverage 83.27% 83.87% +0.59%
- Complexity 845 846 +1
============================================
Files 37 37
Lines 2476 2480 +4
============================================
+ Hits 2062 2080 +18
+ Misses 414 400 -14
Continue to review full report at Codecov.
|
src/RawCodeCoverageData.php
Outdated
| return $coverage; | ||
| } | ||
|
|
||
| private function __construct() |
There was a problem hiding this comment.
Thank you formaking the constructor private and for adopting the concept of named constructors. However, the named constructor must not first create an object and then assign data to it. Instead, the named constructor should pass its parameter, maybe after mapping or processing it, to the constructor.
src/ProcessedCodeCoverageData.php
Outdated
| */ | ||
| private $lineCoverage = []; | ||
|
|
||
| public static function __set_state(array $serialisedData) |
There was a problem hiding this comment.
Why do you think we need a __set_state() method? A value object must not know about persistence or serialization.
There was a problem hiding this comment.
It's because of https://github.com/sebastianbergmann/php-code-coverage/blob/master/src/Report/PHP.php#L37
As this is now an object and not an array, the output from var_export automatically includes a call to __set_state which means it needs to exist as a method for the generated code to continue working.
I could change the PHP export to use serialize/unserialize instead of var_export?
There was a problem hiding this comment.
Do not worry about the PHP report. We can always bump the major version number and change the output of the PHP report.
There was a problem hiding this comment.
__set_state has been removed
… compatibility with data structure changes
dvdoug
force-pushed
the
encapsulate_processed_data
branch
from
118ec2d to
01033e4
Compare
Hello @sebastianbergmann
This the next piece of work for #380. Like the previous PR, this doesn't add new functionality, it merely reworks some of the existing code to use an object instead of an array.
I've also (as a seperate commit) added in the named constructors you requested in #748