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

Code concept of special classes #84

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

safatshahin
Copy link
Contributor

POC of special classes

@safatshahin
Copy link
Contributor Author

This change creates a class for creating reference records and has the question manager handle the creation as a concept. If this change is good, I can start doing similar or related changes for special elements from questionlib and other parts of core_question.

@safatshahin safatshahin force-pushed the master_MDL-75129-api-for-reference branch from c2a5ae8 to 5552699 Compare August 24, 2022 14:26
@timhunt
Copy link
Collaborator

timhunt commented Aug 25, 2022

Safat, I think this is heading in the right direction, but there are serveral things that I would do differently.

  1. I think question_references are sufficiently separate from questions, that I would have a manager class called question_refererence_manager to look after them. (We will probably need the name question_manager for something else later.)
  2. public array $questionreference is a horrible way to store the data in the class. Why would you want to have to write $this->questionreference['usingcontextid'] instead of $this->usingcontextid? If you have separate fields, then each field can have the correct type specified, and a PHPdoc to explain what it means. For a key data object like the most important thing is for it to be easy for all developers to undersstand how this object represents the thing it represents.
  3. I would put the crud operations on the manager class, rather than using something like the active record pattern on individual objects. (E.g. look at the file_storage API). This makes it pssible to have API methods like efficient bulk delete. Yes. No DB operations in the
  4. I think for the references themselves, I think we need a class hierarchy like this:
class question_reference_location {
    public $usingcontext; // Make this the actual context object, I think.
    public $component;
    public $questionarea;
    public $itemid;
    public function __contruct($usingcontext, $component, $questionarea, $itemid) { ... }
    public function __toString() { ... } ... useful for debug display.
}
class question_reference extends question_reference_location {
    public $id;
    public $questionbankentryid;
    public $version;
    // Constructor, etc.
}
class question_set_reference extends question_reference_location {
    public $id;
    public $questionscontextid;
    public $filtercondition; // Should this be the JSON-decoded value?
    // Constructor, etc.
}

(https://github.com/moodleou/moodle-filter_embedquestion/blob/main/classes/embed_id.php is a similar class I created for something.)

@safatshahin safatshahin force-pushed the master_MDL-75129-api-for-reference branch from 5552699 to f8c7683 Compare September 6, 2022 01:55
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

Successfully merging this pull request may close these issues.

None yet

2 participants