marcusgreen / moodle-tool_driprelease

Drip (automate) the availability of moodle course activities over time
3 stars 1 forks source link

Incorrect function naming in lib.php #2

Open danmarsden opened 4 days ago

danmarsden commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 <your@email.com>
 * @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