User tests: Successful: Unsuccessful:
Deprecates JLanguageMultilang::isEnabled()
in favour of JPluginHelper::isEnabled('system', 'languagefilter')
(like all other plugins) and replaces the deprected method in all code base.
None.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_admin com_categories com_contact com_content com_fields com_finder com_languages com_menus com_modules com_newsfeeds com_users Templates (admin) Front End com_config |
Labels |
Added:
?
|
I don't agree. In that Logic we could use more method for other check that components, plugins are enabled. JTagsHelper::isEnabled, JContentHistoryHelper::isEnabled and so on.
I don't think we should add "special" cases.
Also this removes one db query is the cases the plugins are already fetched from the db.
I would argue that the meaning of JLanguageMultilang::isEnabled()
is more explicit to the reader than what you are proposing to replace it with. This makes the code more readable and understandable.
A better "fix", in my opinion, would be to simply rewrite JLanguageMultilang::isEnabled()
as
public static function isEnabled()
{
return JPluginHelper::isEnabled('system', 'languagefilter');
}
well we can do just that, but still i think we should not make special cases with that argument.
Having more methods that do the same thing is IMHO somewhat confusing if you look at the code as a whole.
my point is: as it is now you have
(side note: i actually thinking of having an JExtensionHelper to reduce this 3 db queries in all pages to just one)
anyway, i thing as a whole is more easy to understand if we always use the same methods, not making special cases with the argument that is more readble. Yes, is more readble if you just look at this case, but if you look at code as a whole, IMHO is not.
And what differenciates this case from all other plugins/components/libraries? Are we going to make "special cases" for all of then? At which point we decide: "yes, this needs a special method to check if enabled because is used x times?"
Of course as always, this is only my opinion.
We use JPluginHelper::isEnabled()
, all plugins included, 32 times in core. 12 of them for content vote, 4 for editors, the others being unique or twice.
We use JLanguageMultilang::isEnabled()
70 times in core. It touches almost all aspects of Joomla, contrary to the ones above.
I guess these are the figures that count. I maintain my position about this PR.
sure @infograf768, still IMHO there should not be special cases, expect if not possible otherwise.
anyway, i have no feelings about code, i propose what i think is more correct, if the community does not accept it, fine by me, if accepts it, fine by me also.
Since the two people that comment in this PR (which i appreciate) seem to be against it, if in some days no one agree/comment or a mantainer rejects it: Fine, i will make a PR to go the alternative way described above.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-12-14 22:33:32 |
Closed_By | ⇒ | andrepereiradasilva |
TBH, I see no use for this. We created
JLanguageMultilang::isEnabled()
to simplify code.