Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect function naming in lib.php #2

Open
danmarsden opened this issue Oct 23, 2024 · 3 comments
Open

Incorrect function naming in lib.php #2

danmarsden opened this issue Oct 23, 2024 · 3 comments

Comments

@danmarsden
Copy link

frankenstyle naming violations in the lib.php file eg:
function manage_selections

should be:
function tool_driprelease_manage_selections

and various others in the same file

Note: this is typically an approval blocker.

@marcusgreen
Copy link
Owner

Woiuld it make as much sense to put those routines into a lib class as to create the long function names, and if so should I err on making static calls i.e. lib::get_modules, or create some instances of the lib class. Just looking for some general guidance. Or alternatively just go for the long frankenstyle names (which has the attraction of being straightforward)

@danmarsden
Copy link
Author

My personal preference is to only use lib.php for core hooks or old style callbacks and move all other functions into a namespaced class but that's not a requirement for approval

@marcusgreen
Copy link
Owner

marcusgreen commented Oct 23, 2024

I think we are thinking alike, adding the full prefix in the lib.php makes for very long function names.
I was just experimenting with this

namespace tool_driprelease;

/**
 * Class lib
 *
 * @package    tool_driprelease
 * @copyright  2024 YOUR NAME <[email protected]>
 * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
 */
class lib {
    /**
     * Process the checkbox selections and upsert the database records
     *
     * @param \stdClass $fromform
     * @param int $dripreleaseid
     * @return int $insertedcount // For future testing purposes.
     */
    public function manage_selections(\stdClass $fromform, int $dripreleaseid): int {
        global $DB;
        if (!isset($fromform->activitygroup)) {
            return 0;
        }
etc etc etc

I know it's not required for approval, but I will probably only re-visit this code once so it would be good to get it ready for the future

marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
marcusgreen added a commit that referenced this issue Oct 24, 2024
Triggerd by review for plugins database here
#2
Move functions into a new driprelease class to conform to more
modern php and avoid potential name clashes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants