Skip to content

refactor: replace anonymous class with bound closure in PropertiesTrait - #10504

Open
paulbalandan wants to merge 1 commit into
codeigniter4:developfrom
paulbalandan:properties-trait-get-object-vars
Open

refactor: replace anonymous class with bound closure in PropertiesTrait#10504
paulbalandan wants to merge 1 commit into
codeigniter4:developfrom
paulbalandan:properties-trait-get-object-vars

Conversation

@paulbalandan

Copy link
Copy Markdown
Member

Description
This one needs a bit of decision.

This refactors PropertiesTrait::getPublicProperties() to use a bound Closure instead of an anonymous class in order to delete the 1 remaining missingType.iterableValue error. The other solution was to use reflection similar to getNonPublicProperties() approach, but here's the bench:

200k iterations each, on an object with 5 public + 1 protected + 1 private property:

Approach Time Per call
Anonymous class 0.2721s / 0.2815s ~1.4μs
Bound closure 0.3541s ~1.77μs
Reflection 1.2148s / 1.2236s ~6.1μs

Bound closure is ~26% slower than the anonymous class but ~3.5x faster than reflection.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan paulbalandan added the refactor Pull requests that refactor code label Aug 29, 2026

@datamweb datamweb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks good.

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is minimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Pull requests that refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants