auto-generate some properties #1826
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1826 +/- ##
============================================
+ Coverage 49.09% 49.14% +0.04%
+ Complexity 764 758 -6
============================================
Files 1642 1642
Lines 59624 59749 +125
Branches 26683 26683
============================================
+ Hits 29275 29364 +89
- Misses 30349 30385 +36
Continue to review full report at Codecov.
|
| { | ||
| $this->type = $type; |
There was a problem hiding this comment.
Do want just a list of these types, or do you want a proper validation?
There was a problem hiding this comment.
This object is a simple data container. The validation has to be setup in the factory.
There was a problem hiding this comment.
Data should be validated as it enters the place of usage (i.e. the class), rather than assume it is validated somewhere else; that's why value objects are useful; encapsulate validation of values in a value object, make it immutable so it can never accidentally become invalid (if you want to change the value, you create a new object always), and there is implicit trust that the value object is in a consistent, valid and invariant state.
Specifically in this case, it was a question really: is $type really an enumeration? Can the value be validated?
There was a problem hiding this comment.
I added the validation for the type property in the factory, like all other validation we do.
The type property is not really an enum. The possible values are limited by the external libraries I created and used here and by the validation browscap-php does.
| { | ||
| $this->type = $type; |
There was a problem hiding this comment.
Same - do no validation done here to check $type is valid
|
@asgrim @jaydiablo Could you review this again? |
| */ | ||
| public function testBuildWithWrongDeviceType() : void | ||
| { | ||
| $this->expectException(\UnexpectedValueException::class); |
There was a problem hiding this comment.
Just a minor nitpick, but ideally expectException* should be just before where the exception happens, rather than at the start of the method. No need to change anything here, but just maybe more logical :)
|
Thanks @mimmi20 👍 |
|
Thanks |
With this PR the following properties are removed from the source files and genererated during the build process
Additional to that in the source files the strings "true" and "false" were changed to the boolean values true or false, some integer properties (e.g. Browser_Bits, CssVersion) have been converted to int.
The Models and Tests have been updated for this. Some missing tests were added too.
This PR should fix #1433.