[#16412] - Remove unnecessary condition
- Closed
- 29 Apr 2018
- Medium
- Build: staging
- # 16412
- Diff
- nvyush:patch-1
- 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:
The check !empty($this->item) is unneeded at this point.
It is possible to join the statements of if conditions at the lines 72 and 77.
Pull Request for Issue #16409.
Summary of Changes
Unnecessary condition is removed.
The statements of equivalent conditions is joined.
Testing Instructions
Code review.
Expected result
Actual result
Documentation Changes Required
Not needed.
| Status | New | ⇒ | Pending |
| Category | ⇒ | Front End com_content |
Well, but check !empty($this->item) is not needed and can be removed. If $this->item is empty at this point, all checks $this->item->id above this line will be wrong. Are you agree?
I can update the commit.
| Labels |
Added:
?
|
||
Imho, the isset should indeed be sufficient, because it will also return false if the item doesn't exist. So we gain nothing by explictiely checking the existence of the item prior to it.
I tried to restart the drone test. looks like an unrelated error to me.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16412.
For new items the ID is always NULL. Even by saying a_id=0 in the url. So why not joining the two if-conditions?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16412.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-04-29 03:50:33 |
| Closed_By | ⇒ | Quy |
Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/16412
| Closed_Date | 2018-04-29 03:50:33 | ⇒ | 2018-04-29 03:50:34 |
| Closed_By | Quy | ⇒ | joomla-cms-bot |
See PR #20254
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16412.
Not sure if that is accurate.
Because the first check
!empty($this->item->id)is only true when the item already has an id set and other than 0/false (means we edit an existing item).The second check
!empty($this->item) && isset($this->item->id)will be true if the item is valid, but the ID may also be 0 (means it can also be a new item).