Use sprintf in exception messages #2
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2 +/- ##
============================================
+ Coverage 94.87% 95.00% +0.12%
+ Complexity 19 18 -1
============================================
Files 2 2
Lines 39 40 +1
============================================
+ Hits 37 38 +1
Misses 2 2 ☔ View full report in Codecov by Sentry. |
src/BackedEnum.php
Outdated
| @@ -28,17 +28,17 @@ public static function fromName(string $fqn, string $keyName): \BackedEnum | |||
| */ | |||
| public static function tryFromName(string $fqn, string $keyName): ?\BackedEnum | |||
| { | |||
| if (is_a($fqn, \BackedEnum::class, true) === false) { | |||
| throw new InvalidArgumentException('It is only possible to get names of backedEnums, "' . $fqn . '" provided'); | |||
| if (!is_a($fqn, \BackedEnum::class, true)) { | |||
There was a problem hiding this comment.
Although in this case, the function always returns a boolean, native PHP functions are notorious for returning mixed types. the NOT operator (!) does not only check against a type, but also casts the value implicitly to a boolean, so if the return value of a function is 0 or [] it will also result in the exception being thrown. As it is complex to explain in code reviews when a NOT operator is allowed and when it is not 😛, I try to give a good example and never use it at all, even though in this case it would be possible to use it without causing any unwanted bugs.
There was a problem hiding this comment.
native PHP functions are notorious
There is a solution for this problem: https://github.com/thecodingmachine/safe
| } | ||
|
|
||
| if (!defined("$fqn::$keyName")) { | ||
| if (!defined("{$fqn}::{$keyName}")) { |
There was a problem hiding this comment.
Is this a code style or optimization I'm not aware of or just a personal preference?
There was a problem hiding this comment.
- curly braces make it safer
- easier to spot bugs
- explicit variable start and end
- and finally: use
sprintf😉
https://www.php.net/manual/en/language.types.string.php#language.types.string.parsing.complex
| if (!defined("$fqn::$keyName")) { | ||
| if (defined("{$fqn}::{$keyName}") === false) { |
There was a problem hiding this comment.
This is a new ✨ === false
...
andis_areturns a boolean, so use!and safe complex curly syntax