[#44163] - [5.3] Refactoring AbstractView::getName()
- Closed
- 1 Oct 2024
- Medium
- Build: 5.3-dev
- # 44163
- Diff
- Hackwar:5.3-view-getName
User tests: Successful: Unsuccessful:
Summary of Changes
While reviewing the AbstractView class, I stumbled upon the use of the reflection class in the getName() method of AbstractView. This PR refactors the method to not use reflection.
Testing Instructions
Codereview
Actual result BEFORE applying this Pull Request
Everything should work identical to before.
Expected result AFTER applying this Pull Request
Everything should work identical to before.
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
PR-5.3-dev
|
||
What's wrong with reflection?
To be honest, it just feels overkill to use this and not exactly performant when you could also simplify it like I did above. I'm not married to this PR. If you disagree, please close it. It is just a proposal.
Ok, then I feel free to close as I see little to none benefit and it should be covered with proper unit tests to make sure we do not introduce here a regression. You can reopen anytime if this gets decent coverage as speed improvements are always welcome. Even when I doubt that we gain here much.
But keep up the good work with the cleanup as this is very important work. Thanks!
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-10-01 14:03:32 |
| Closed_By | ⇒ | laoneo |
This one scares me more than the others - mainly because we've treated this differently to the model and controllers getName().
I think I at least want to see a unit test for this one (sorry) - both for a namespaced and non-namespaced child class.