count() used in the loop condition

[krunal265]

The use of count() inside a loop condition is not allowed; assign the return value to a variable and use the variable in the loop condition instead.

[krunal265]

patch added.

[johnbillion]

Thanks for the patch @krunal265 . Have you tested this change? The $comments array gets populated inside the while which means this change will break this functionality because the count is no longer updated with each iteration.

[SergeyBiryukov]

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Do you get any PHPCS warnings from this file? When running WPCS checks for WordPress core with the default phpcs.xml.dist​ file, wp-admin/includes/dashboard.php appears to pass all the checks successfully in my testing. Could you clarify how you run the PHPCS checks?

As noted above, the $comments array is changed inside the loop, so the value needs to be recalculated on each iteration, and the patch may not work as expected. That said, I agree that the condition could probably be simplified.

[krunal265]

@johnbillion, @SergeyBiryukov Thank you for replying and looking into this ticket.
Yes, I have tested this change and it's working fine. Every new comment is displayed on the dashboard after changing this.

[krunal265]

Hello @SergeyBiryukov, @johnbillion
Any further updates regarding this issue?

[SergeyBiryukov]

Replying to krunal265:

Yes, I have tested this change and it's working fine. Every new comment is displayed on the dashboard after changing this.

Thanks! I think an answer to the questions from comment:2 would be helpful to move the ticket forward, as the file passes all WPCS checks for core and it's not quite clear to me what is the exact issue we're trying to fix here:

Do you get any PHPCS warnings from this file? When running WPCS checks for WordPress core with the default phpcs.xml.dist​ file, wp-admin/includes/dashboard.php appears to pass all the checks successfully in my testing. Could you clarify how you run the PHPCS checks?

The patch as is does not look correct to me, as $comments is set to an empty array at the beginning of the function​, so count( $comments ) would always be zero at that point, and the condition becomes redundant, though it looks like the function still works due to the break statements inside the loop.

If we need to remove the count() call from the loop condition here, we could change the loop to do ... while instead, see 56499.diff​. I'm not 100% sure this change is necessary, but it might be a readability improvement, so I'm moving this to 6.1 for further discussion.

[audrasjb]

As the patch and the issue itself are still being discussed, and since beta 1 is in a few days, let's move this ticket to Future Release milestone.

@krunal265 there is still some questions to answer about the potential issue underlined in this ticket, that's why I'm moving it to Future Release. We can still move it to the next major (6.2) if the issue is clarified :)

[krunal265]

@audrasjb, We can use do...while loop in this issue instead of while loop. From this change, coding standards and readability will improve. Because the use of count() inside a loop condition is not allowed; assign the return value to a variable and use the variable in the loop condition instead.

[SergeyBiryukov]

I believe 56499.diff​ addresses the request here.