[#1839] - JText: Changed conditional to make it easier to read
- Closed
- 4 Jun 2014
- Medium
- Build: master
- # 1839
- Diff
- Hackwar:jtext
- Error default This pull request is targeted at the master branch. Pull requests should no longer be merged to master. Details
User tests: Successful: Unsuccessful:
I had serious problems understanding this conditional, so I simplified it a bit.
Old behavior:
1a. Check if there is a comma in the string. If not, return false.
2a. Compare that false to "false", resulting in "true".
3a. Negate that "true", resulting in false. Skip this block.
1b. Check if there is a comma in the string. If there is, return true.
2b. Compare that "true" against "false", resulting in "false".
3b. Negate that "false", resulting in true. Executing this block.
New behavior:
1a. Check if there is a comma in the string. If not or the comma is at the first position, return false or 0, which is both falsey. Skip this block.
1b. Check if there is a comma in the string. If there is and the comma is not at the first position, return an int > 0, which evaluates to true. Execute this block.
The only difference to before is, that you can't have a key with a leading comma. As far as I can see, this is never the case. At the same time, this code is a LOT more readable.
It is still valid.
And with the words of Cato the Elder: Ceterum censeo Joomlacode esse delendam (I believe that Joomlacode has to be destroyed. ;-) )
And with the words of Cato the Elder: Ceterum censeo Joomlacode esse delendam (I believe that Joomlacode has to be destroyed. ;-) )
Most agree on that and we're working on it. You can help: https://github.com/joomla/jissues
phproberto
- | Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-06-04 23:59:02 |
Thanks for your contribution - At this time we are only using github as the place to submit code fixes, the actual reporting of issues and testing fixes is still taking place on Joomlacode.
As it has been some time since you opened this issue can you please confirm that it is still valid with the current Master or Joomla 3.2 beta. If it is no longer valid then please can you close this issue. Otherwise please can you:
1) Open an item on the Joomlacode tracker in the appropriate area.
CMS Bug Reports: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemBrowse&tracker_id=8103
CMS Feature Requests: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemBrowse&tracker_id=8549
2) After submitting the item to the Joomlacode tracker, add a link to the Joomlacode tracker item here and make sure that you add a link to this GitHub issue or pull request on the joomlacode tracker item.