Skip to content

[5.3] Remove useless empty ($form) check #45039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 5.3-dev
Choose a base branch
from

Conversation

joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

The loadForm method from FormBehaviorTrait (which is used by our models) always return a Form object on success and throw Exception on failure, so all empty($form) check in our model classes are useless, it never returns false, thus should be moved from models code.

Testing Instructions

This will require careful code review from our testers and maintainer.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works, slight faster because the useless code is removed, thus not being executed.

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

@joomdonation joomdonation changed the title Remove useless empty ($form) check [5.3] Remove useless empty ($form) check Feb 28, 2025
@exlemor
Copy link

exlemor commented Aug 23, 2025

@joomdonation Can you please provide testing instructions - how can we test this? - 58 files in code review may not be enough.

Love to include it in PBF which is today :)

@joomdonation
Copy link
Contributor Author

Thanks @exlemor. This is too late to be included in 5.3, so please ignore it for now. I will try to make it for Joomla 6 later.

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

Successfully merging this pull request may close these issues.

6 participants