Enabled Strict types and improved type hinting #2
Conversation
…y followed. - Added missing return types so callables will know what will be returned - Reformated code so its more readable with && and || signs. - Re-arranged the simplicity of the || checks. - Removed intval() and did (int) casting. Which performs up to 6 times faster. - Returned void for addLinkHeaders since its private and the return is never used. - For LinkHeaders.php I've removed the null-operator (?). This could cause an error whenever the null was passed. Now it can't throw an error anymore due to the foreach which was a few lines later. - Renamed variable instead of rewriting the variable (bad coding behavior) - Tested everything in the test folder to assure everything complies.
src/Data/LinkHeaders.php
Outdated
| $value = $matches['value'] ?? '1'; | ||
|
|
||
| if ($key === 'rel') { | ||
| $rel = $value; | ||
|
|
||
| continue; | ||
| } | ||
| $attributes[$key] = $value ?? true; | ||
| $attributes[$key] = $value; |
There was a problem hiding this comment.
In this case this would also impact the rel, adding rel="1" to the headers if the value is empty/not matched, are there any cons to adding the fallback value here?
There was a problem hiding this comment.
You're right. I was tracing the line of how this would work. I've got it at the wrong end. My IDEA mislead me. I'll revert the change! :)
src/Data/LinkHeaders.php
Outdated
| { | ||
| $handledHashes = []; | ||
|
|
||
| foreach ($this->getLinkProvider()->getLinks() as $link) { | ||
| /** @var Link $link */ | ||
| $hash = md5($link->getHref(), serialize($link->getRels())); | ||
| if (! in_array($hash, $handledHashes)) { | ||
| $hash = md5($link->getHref(), true); // Previous the second parameter was: serialize($link->getRels()) which gives a string, where the second parameter excepts a boolean. |
There was a problem hiding this comment.
You're right! This is what it should've been, good catch 🙂
| $hash = md5($link->getHref(), true); // Previous the second parameter was: serialize($link->getRels()) which gives a string, where the second parameter excepts a boolean. | |
| $hash = md5($link->getHref() . serialize($link->getRels())); |
src/Data/LinkHeaders.php
Outdated
| { | ||
| return trim(collect($this->getLinkProvider()->getLinks()) | ||
| ->map([static::class, 'linkToString']) | ||
| ->map([self::class, 'linkToString']) |
There was a problem hiding this comment.
My preference here is to use static since if it is ever needed this class could be extended, overriding the linkToString function and then it would call the function on the class extending this one instead of still executing the function of this class itself
There was a problem hiding this comment.
Fair, I changed the statics to self and then most self back to static since there was no need. Forgotten statement. Also reverting this change, sorry!
|
The pint codestyle action seems to be failing, you should be able to fix that with |
- Adjusted code based on feedback. - Ran command to fix-style. - Added return types to test cases.
Nice, done that also. Added the new commit, have a look! :) |
indykoning
changed the title
|
Awesome! i've merged the changes and will add them to the next release 🙂 |
declare(strict-types=1);to ensure that every type is strictly followed.&&and||signs.||checks.intval(...)and did(int)casting. Which performs up to 6 times faster.?. This could cause an error whenever thenullwas passed. Now it can't throw an error anymore due to the foreach which was a few lines later.