Skip to content

[4.4] Introduce local variables to replace repeating method calls#40224

Merged
laoneo merged 5 commits intojoomla:4.4-devfrom
joomdonation:introduce_local_variables
Mar 29, 2023
Merged

[4.4] Introduce local variables to replace repeating method calls#40224
laoneo merged 5 commits intojoomla:4.4-devfrom
joomdonation:introduce_local_variables

Conversation

@joomdonation
Copy link
Copy Markdown
Contributor

Pull Request for Issue # .

Summary of Changes

This PR introduces local variables $app and $db to replace the repeating calls $this->getApplication() and $this->getDatabase(). phpstorm has a built-in function called Refactor -> Introduce Variable allows us to do this quickly. We do not have a clear rule for this, but I think doing that would make code more cleaner and a little faster.

Testing Instructions

  1. Use Joomla 4.4
  2. Apply patch
  3. Try to login, check on Remember Me checkbox, make sure it is working OK
  4. Code review would be great, too.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works with cleaner code and a little faster.

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

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Mar 28, 2023

I'm not sure if there should be a blank line before the if blocks. Beside that it looks good! Thanks.

@laoneo laoneo merged commit c9c478a into joomla:4.4-dev Mar 29, 2023
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Mar 29, 2023

Thanks!

@laoneo laoneo added this to the Joomla! 4.4.0 milestone Mar 29, 2023
@joomdonation joomdonation deleted the introduce_local_variables branch March 29, 2023 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants