[wp-trac] [WordPress Trac] #56499: count() used in the loop condition

[WordPress Trac]

#56499: count() used in the loop condition
-------------------------------------+-------------------------------------
 Reporter:  krunal265                |       Owner:  (none)
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  6.1
Component:  Comments                 |     Version:  6.0
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:  administration,
  reporter-feedback                  |  coding-standards
-------------------------------------+-------------------------------------
Changes (by SergeyBiryukov):

 * focuses:  coding-standards => administration, coding-standards
 * component:  Administration => Comments
 * milestone:  Awaiting Review => 6.1


Comment:

 Replying to [comment:4 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 [source:tags/6.0/phpcs.xml.dist
 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
 [source:tags/6.0.2/src/wp-
 admin/includes/dashboard.php?marks=1050,1061#L1048 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
 [attachment:"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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56499#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform