Update library for PHP 7.2 #112
Conversation
cnovak
left a comment
cnovak
left a comment
There was a problem hiding this comment.
Not sure why this is being changed, not description on commit, see comments on sections, but true for all changes. These changes seem to only check for variables being empty, not whether they are arrays or empty arrays.
| { | ||
| // Find the credential with the max issuedAt attribute which isn't expired. | ||
| if (count($credentials) > 0) { | ||
| if (!empty($credentials)) { |
There was a problem hiding this comment.
This only checks to make sure the var is not empty, but isn't validating that it is an array. Should this validate it is an array before trying to sort?
There was a problem hiding this comment.
Here we can be sure credentials is an array, since its required by the function declaration. This instance of count() doesnt have to be removed, but I generally went with using empty() instead of count ==0 or count > 0
|
|
||
| // Some apps may be misconfigured and need to be populated with their apiproducts based on credential. | ||
| if (count($obj->apiProducts) == 0) { | ||
| if (empty($obj->apiProducts)) { |
There was a problem hiding this comment.
Is there a reason we need to change this for 7.2? Are these vars not arrays?
There was a problem hiding this comment.
They probably are arrays, but the general idea here was to use empty() instead of comparing count to 0. This was the strategy employed for drupal 7 core as well, see https://www.drupal.org/project/drupal/issues/2885610
|
Sorry, should have included an explanation! |
3 participants
In places where count() was being used to test for empty (count($var) == 0) or not empty (count($var) > 0)) they are replaced with empty(). In places where an actual count was needed, the line was no modified.