-
Bug
-
Resolution: Fixed
-
Minor
-
4.3
-
MOODLE_403_STABLE
-
MOODLE_403_STABLE
-
MDL-79351-master -
-
1
-
HQ 2023 Sprint I3.3 Moppies
In the file completion/classes/form/form_trait.php, a trait is used for global and course activity completion forms.
The current get_cm method in this trait scans the parent object for a private _cm attribute. This kind of self-discovery of private attributes in traits is a bad practice.
Traits are tricky and should be as self-contained as possible. A trait method should not try "smart" things to access potential parent attributes. This will result in a dependant logic in trait and a completely unmaintainable code in the long run.
There are two valid approaches to solving this kind of situation:
Approach 1: Use an abstract method (not recommended in this specific case)
The trait should not try to scan the parent for information (and, of course, it should not throw an exception if it is incapable of it). Instead, the get_cm can be an abstract method the use class must implement. This way, the use class is forced to implement the proper method.
Approach 2: Use a dependency inversion (recommended in this case for consistency with MDL-78530)
Instead of having a get_cm method, add a new parameter to the methods that require the course. This way, the course dependency is on the use class, not the trait. This means adding a new $cm param to definition_after_data_completion because it is the only method that requires the attribute.
This is the approach applied in MDL-78530 to avoid having a get_course method.