WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 2 months ago

#53988 new enhancement

remove duplicate code in wp_list_filter()

Reported by: pbearne Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

The code in wp_list_filter() is the same as the code in wp_filter_object_list() with no $field so I have created a patch to wrap wp_filter_object_list removing duplicated code

Attachments (1)

53988.diff (1.3 KB) - added by SergeyBiryukov 2 months ago.

Download all attachments as: .zip

Change History (8)

This ticket was mentioned in PR #1615 on WordPress/wordpress-develop by pbearne.


2 months ago

  • Keywords has-patch added

#3 follow-up: @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.9

Thanks for the PR!

It looks like wp_filter_object_list() used to wrap wp_list_filter() before [38928] / #37128.

So I think I would prefer reinstating that. Doing it the other way around might have more unnecessary if ( $field ) checks.

Last edited 2 months ago by SergeyBiryukov (previous) (diff)

#4 in reply to: ↑ 3 @SergeyBiryukov
2 months ago

Replying to SergeyBiryukov:

So I think I would prefer reinstating that.

Scratch that, it looks like that would require creating a second WP_List_Util instance, which might have a performance or memory impact, depending on the size of the list.

#5 @SergeyBiryukov
2 months ago

Some more history here: [14108], [15686], [17427], [38928].

#6 @SergeyBiryukov
2 months ago

The unit tests for wp_list_filter() currently fail with this PR:

There was 1 failure:

1) Tests_Functions_wpListUtil::test_wp_list_filter with data set "invalid operator" (array(stdClass Object (...), stdClass Object (...)), array('bar'), 'XOR', array())
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
+    0 => stdClass Object (...)
+    1 => stdClass Object (...)
 )

/var/www/tests/phpunit/includes/abstract-testcase.php:813
/var/www/tests/phpunit/tests/functions/wpListUtil.php:373

#7 @SergeyBiryukov
2 months ago

The failure is because WP_List_Util::filter() performs validation of the $operator argument, but in case of an invalid operator it just returns early and does not assign the $this->output value that wp_filter_object_list() returns.

This means that wp_filter_object_list() and wp_list_filter() treat an invalid operator differently:

  • wp_filter_object_list() returns the original array.
  • wp_list_filter() returns an empty array.

53988.diff fixes that and the tests seem to pass now, though it might be up for discussion whether the current behavior is a bug or a feature.

Note: See TracTickets for help on using tickets.