Simplify some ternary operations using elvis operator and remove unnecessary parentheses in libraries/legacy#13240
Simplify some ternary operations using elvis operator and remove unnecessary parentheses in libraries/legacy#13240rdeutz merged 9 commits intojoomla:stagingfrom frankmayer:elvis-and-unneeded-parentheses-in-libraries-legacy
Conversation
…cessary parentheses
…es-legacy # Conflicts: # libraries/legacy/model/legacy.php
|
Conflicts resolved. Pls check and merge. |
| @@ -103,10 +103,10 @@ public function __construct($options) | |||
| $this->_table = $options['table']; | |||
| $this->_field = (isset($options['field']) && $options['field']) ? $options['field'] : 'catid'; | |||
| $this->_key = (isset($options['key']) && $options['key']) ? $options['key'] : 'id'; | |||
There was a problem hiding this comment.
Remove parentheses in lines 104 and 105.
There was a problem hiding this comment.
If I am not mistaken, in these cases, you can't just simplify it. Since you need the isset() to make sure the value is set... And if simplifying the ternary with the result of (isset($options['field']) && $options['field']) you get a 1 in case of true, not the value of $options['field']
There was a problem hiding this comment.
Like this: $this->_field = isset($options['field']) && $options['field'] ? $options['field'] : 'catid';
There was a problem hiding this comment.
Ah, you meant removal of parentheses... 😄 I was reading it like simplifying the ternary operation...
There was a problem hiding this comment.
Sorry. I updated the comment after my initial posting.
There was a problem hiding this comment.
will you remove the parenthesis then?
libraries/legacy/model/list.php
Outdated
| } | ||
|
|
||
| if (($cur_state != $new_state) && $new_state !== null && ($resetPage)) | ||
| if (($cur_state != $new_state) && $new_state !== null && $resetPage) |
There was a problem hiding this comment.
Remove parentheses ($cur_state != $new_state).
…n-libraries-legacy' into elvis-and-unneeded-parentheses-in-libraries-legacy
| $options = array( | ||
| 'defaultgroup' => ($group) ? $group : (isset($this->option) ? $this->option : JFactory::getApplication()->input->get('option')), | ||
| 'cachebase' => ($client_id) ? JPATH_ADMINISTRATOR . '/cache' : $conf->get('cache_path', JPATH_SITE . '/cache'), | ||
| 'defaultgroup' => $group ?: (isset($this->option) ? $this->option : JFactory::getApplication()->input->get('option')), |
There was a problem hiding this comment.
Align =>. (Sorry I missed these before 😦 )
There was a problem hiding this comment.
I think there are many such cases in lots of files, but lets do those in another PR...
I'll do the ones you have mentioned already, but lets try to get this RTC ;)
There was a problem hiding this comment.
I think that there might already be another PR for those cases. If not, we can do that in new ones.
| public function delete($pk = null) | ||
| { | ||
| $k = $this->_tbl_key; | ||
| $pk = (is_null($pk)) ? $this->$k : $pk; |
|
I have tested this item ✅ successfully on 99ec6fc This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13240. |
1 similar comment
|
I have tested this item ✅ successfully on 99ec6fc This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13240. |
|
RTC after two successful tests. |
Summary of Changes
This PR is part of a set to try to separate some of the changes done in one of my previous batch PR's for the libraries/legacy directory, which is still on hold (#12220).
Once the new set is merged it will hopefully reduce the changes in that PR, so it can be reviewed easier and finally be merged.
The changes in this PR should be fairly easy to review. In hope that this will get merged quickly. ;)
Testing Instructions
None, should not change behavior
Documentation Changes Required
None.