?
avatar Hackwar
Hackwar
15 Dec 2016

Please review the names of the new methods that are being introduced with the fields feature. JFormField got three new methods and the names of all of those methods are not self-explanatory. I expect that you look at a method name and together with the class name can deduce what this method is doing, like JRouter::parse() or JViewLegacy::display().

If that really, REALLY is not possible, then at least add extensive comments to the methods. In #12968 several component helper classes get a new method named "getRealSection". That name has absolutely zero meaning for me. Even when I look at the code, I have no idea what this is for. But when I read the doc block, I get the following

 	/**
	 * Map the section for custom fields.
	 *
	 * @param   string  $section  The section to get the mapping for
	 *
	 * @return  string  The new section
	 *
	 * @since   __DEPLOY_VERSION__
	 */

This doc block does not describe anything. What is a section? What is this for? Is this made for com_finder or K2? What does it map the section from and to? If this code is released like this, in a years time no one will know what this does and/or what this is for. Neither from the code nor from the doc block it is understandeable that this takes the context of a form and based on the component and front- or backend translates it into the actual form context that is being used. The doc blocks in JFormField are equally non-descriptive.

This is part of a code review of the fields feature. See #13222, #13223 and other connected issues.

avatar Hackwar Hackwar - open - 15 Dec 2016
avatar joomla-cms-bot joomla-cms-bot - change - 15 Dec 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 15 Dec 2016
avatar Bakual Bakual - change - 15 Dec 2016
Title
Fields: Use descriptive method names and comments
[com_fields] Use descriptive method names and comments
avatar Bakual Bakual - change - 15 Dec 2016
Title
Fields: Use descriptive method names and comments
[com_fields] Use descriptive method names and comments
avatar Bakual Bakual - edited - 15 Dec 2016
avatar laoneo
laoneo - comment - 30 Dec 2016

Changed the name in pr #12968. The new functions in JFormField will get removed with pr #13319. So I guess we can close this one then.

avatar Bakual
Bakual - comment - 1 Jan 2017

Closing as we have PRs.

@Hackwar Feel free to reopen if there are still issues.

avatar Bakual Bakual - change - 1 Jan 2017
Status New Closed
Closed_Date 0000-00-00 00:00:00 2017-01-01 17:03:00
Closed_By Bakual
avatar Bakual Bakual - close - 1 Jan 2017

Add a Comment

Login with GitHub to post a comment