[#12220] - Performance 2 (libraries/legacy)
- Fixed in Code Base
- 25 Jul 2017
- Medium
- Build: staging
- # 12220
- Diff
- frankmayer:Performance_2
- Success hound No violations found. Woof! Details
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Error continuous-integration/jenkins/pr-merge This commit cannot be built Details
User tests: Successful: Unsuccessful:
[UPDATED (6.1.2017) - see second paragraph]
Performance (and a bit of cleanup) Batch #2
The changes in this batch are all in files under libraries/legacy.
This PR modifies code to be a bit more performant and also does some cleanup.
I have mostly done work on low hanging fruit. There are still other ways of improving, but whose involve more deep research in the code and probably more drastic changes in order to be implemented.
[Update 6.1.2017]
In order to lighten this PR up, I have introduced 6 sub-PRs, which are easier to digest and mostly with very specific changes. In order to continue with this one, those ones should be tested/reviewed/merged first. After that, I can resolve the conflicts and there should finally be only a few changes left.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Category | ⇒ | Libraries |
=== is more strict than ==, it will validate that something matches both the value and data type (so in that case 0 === '0' would fail but 0 == '0' is OK).
It's useful with some of the string functions where you can have a boolean false and an integer zero return. With == the values are compared to represent the same thing but with === they aren't.
@sovainfo The main issue with == comparison is that sometimes it casts the values in very odd way, for example:
echo (0 == 'asd') ? 'true' : 'false'; // trueWhich is just wrong. On the other hand you really need to be careful with === as if you're comparing different types, it always returns false. This is true especially with all (integer) values coming from DB, which are always strings, but usually integers if you happen to update value of the object:
echo (0 === '0') ? 'true' : 'false'; // false
Thank you all for the feedback. For some reason thought that 'null' would never compare equal to anything, not even 'null'. Maybe that only applies to SQL, requiring you to use 'IS NULL'.
Concerning values coming from the database, expect the driver to make sure the right datatype to be returned. Apparently it doesn't.
Conflicts resolved
I have tested this item
code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
I have tested this item
Code review OK
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
@frankmayer can you please look at the conflicting file?
@jeckodevelopment Thanks, done.
Edited initial post to show which sub-PRs still have to go through, in order to make this one easier to digest.
It would be great if the initial big batch PRs of mine could make it into alpha2. Thanks!
I have tested this item
code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
I have tested this item
code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
I have tested this item
Code review.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12220.
RTC please, thank you
| Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
Thanks for Hint, @frankmayer
rdeutz
- | Milestone |
Added: |
||
| Milestone |
Added: |
||
Merge pls?
| Milestone |
Added: |
||
| Milestone |
Removed: |
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-25 23:48:04 |
| Closed_By | ⇒ | mbabker |
Could you explain when '== 0' is oke and when it needs '=== 0' ?