[#6773] - Simply the code in backend controllers
- Closed
- 8 Dec 2015
- Medium
- Build: staging
- # 6773
- Diff
- joomdonation:patch-7
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
PR Summary
In display method of many backend controllers, we are having this code:
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $id));
$this->setMessage($this->getError(), 'error');These two lines of code is used to set an error message to controller object and display to end-user if users try to edit a record by typing the link directly on browser.
The two lines of code can be replaced with just one line of code:
$this->setMessage(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $id), 'error');This is more logic to me. There is no reason to set error message, then call a command to get the same error message to display.
I made change to few classes as a sample. If people think that this change is valid, I will do the same changes on other classes.
joomla-cms-bot
- | Labels |
Added:
?
|
||
In theory, Yes, it might be BC issue. However, in reality, I don't think there will be any third party code which count on getError/s of these backend controllers.
B/C is not about guessing ;) I don't like it too, but it should be mentioned.
Since this PR deals with controllers of extensions, there is no B/C to keep in mind. That would only be the case if wtouch the library classes.
setError and getError is also already deprecated, so it is a good move to get rid of using them.
Thanks Thomas. So I am correct that I can do the same change for remaining controllers in backend?
Imho, it should be fine. But I don't know why it was done this way originally 
Who know :D, sometime, we still see strange code like that in Joomla core. I will modify the remaining controllers and then PLT can decide to merge it or not.
OK. I completed the modification for all backend controllers. Hope it will get merged :).
To clarify: I'm not against this PR. But if have to disagree with @Bakual
That would only be the case if wtouch the library classes.
Of course libraries are the main suspects, but core elements like mandatory extensions have to take care of B/C too. Hard example: Let's rename a components view.
Of course libraries are the main suspects, but core elements like mandatory extensions have to take care of B/C too. Hard example: Let's rename a components view.
Renaming a view would actually be possible with proper redirects/alias in place. As long as it doesn't break a link.
Our B/C promise regarding API and code is only for the libraries. This is because those classes are intended to be extended by 3rd party extensions.
However the classes from the components are not meant to be extended or directly used by 3rd party extensions. We strongly discourage you from doing that. If you do, you're on your own.
That's why this PR is fine from a B/C point of view. It doesn't break anything.
Taking the discussion to the Mailinglist: https://groups.google.com/forum/#!topic/joomla-dev-cms/GTnqEDMgOwk
zero-24
- | Category | ⇒ | Administration Code style Components |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-12-08 07:06:44 |
| Closed_By | ⇒ | joomdonation |
Possible BC issue. Even if JControllerLegacy doesn't care about it's origin JObject, there might be third party code which counts on
getError/s().