[minor] SCA #26938
[minor] SCA #26938
Conversation
kalessil
commented
Apr 15, 2018
| Q | A |
|---|---|
| Branch? | 2.7 |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | n/a |
- Control flow tweaks
| } | ||
|
|
||
| return false; | ||
| return class_exists($class) || interface_exists($class); |
There was a problem hiding this comment.
while at it, the second call should have false as second argument
| @@ -56,8 +56,7 @@ public function __construct($name, $shortcut = null, $mode = null, $description | |||
| if (is_array($shortcut)) { | |||
| $shortcut = implode('|', $shortcut); | |||
| } | |||
| $shortcuts = preg_split('{(\|)-?}', ltrim($shortcut, '-')); | |||
| $shortcuts = array_filter($shortcuts); | |||
| $shortcuts = array_filter(preg_split('{(\|)-?}', ltrim($shortcut, '-'))); | |||
There was a problem hiding this comment.
No performance gain, just fewer lines of code to maintain.
There was a problem hiding this comment.
In general, performing two distinct operations in two lines of code is easier to understand than performing two distinct operations in one line of code.
The number of lines of code to be maintained is less of an issue than the ability for the code to be easily understood.
I'd say this specific change is better being reverted to the two line version.
There was a problem hiding this comment.
I'll revert, since benefits are questionable.
Regarding the argumentation we should distinguish business logic (where I'll agree with you) and low-level stuff like strings manipulation (which is normally needs diving into in any case).
There was a problem hiding this comment.
@nicolas-grekas @webignition : reverted
|
Thank you @kalessil. |
This PR was squashed before being merged into the 2.7 branch (closes #26938). Discussion ---------- [minor] SCA | Q | A | ------------- | --- | Branch? | 2.7 | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a - Control flow tweaks Commits ------- 877e678 [minor] SCA