Make WordPress Core

Opened 10 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#63216 closed enhancement (fixed)

simplify get_role() method in WP_Roles class

Reported by: dilipbheda's profile dilipbheda Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Role/Capability Keywords: has-patch
Focuses: coding-standards Cc:

Description

This update simplifies the get_role() method in the WP_Roles class by removing an unnecessary else block. The method now directly returns null when the role is not found, improving readability and maintaining consistency with WordPress coding standards.

Change History (6)

This ticket was mentioned in PR #8637 on WordPress/wordpress-develop by @dilipbheda.


10 months ago
#1

#2 @spacedmonkey
10 months ago

Thanks for your ticket @dilipbheda .

There is a rule with WordPress core, that we do not commit refactors for the sake of refactors. There is lots of code and if we refactored for the sake of it, it would be never ending task.

Can you provide more context why this change is needed, other than the code is cleaner. Is there a performance benefit?

Please read this docs for more information.

https://make.wordpress.org/core/handbook/contribute/code-refactoring/

#3 @dilipbheda
10 months ago

Hello @spacedmonkey,

Thank you for the sharing the document.

Reason for this changes

1) When reviewing the get_role() function, I noticed an unnecessary else block. Since the if statement already returns a value, the else can be removed for cleaner code.

2) The get_role() function is used daily and has minimal code, minimizing the risk of regression. So, I created a ticket and a patch for it.

For shorter condition

Initially, I only removed the else block and returned the value directly. However, based on reviewer feedback, I simplified it with a shorter condition.
Here: https://github.com/WordPress/wordpress-develop/pull/8637/commits/7b08649a6402c924fc28c2640e0330d82dba8075

There’s no other reason for this ticket. Feel free to close it if the changes aren’t needed.

Thanks

#4 @SergeyBiryukov
10 months ago

  • Focuses coding-standards added

#5 @SergeyBiryukov
4 weeks ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 61404:

Code Modernization: Replace isset() with null coalescing in WP_Roles::get_role().

Since PHP 7.0 introduced the null coalescing operator, and WordPress now requires at least PHP 7.2.24, isset( $var ) ? $var : null ternary checks can be safely replaced with the more concise $var ?? null syntax.

As some new code using the null coalescing operator has already been introduced into core in recent releases, this commit continues with the code modernization by implementing incremental changes for easier review.

Follow-up to [2703], [61403].

Props dilipbheda, mukesh27, spacedmonkey, SergeyBiryukov.
Fixes #63216. See #58874.

#6 @westonruter
4 weeks ago

  • Milestone changed from Awaiting Review to 7.0
  • Type changed from defect (bug) to enhancement
Note: See TracTickets for help on using tickets.