Changes to course listings in 2.5

Re: Changes to course listings in 2.5

by Marina Glancy -
Number of replies: 9
Picture of Core developers Picture of Moodle HQ Picture of Moodle Workplace team Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers Picture of Testers

Gareth,

if theme can not override format_topics_renderer because of the extra parent, it is a serious problem. And if it is the case we need to raise a tracker issue for that and move discussion there. I have not tried it myself yet so I can't confirm at the moment.

The solution that I see is to make the format_section_renderer_base not abstract and other renderers such as format_topics_renderer will do $this->baserenderer = $this->page->get_renderer(...) in their constructor and instead of calling parent::xxx() will do $this->baserenderer->xxx().

And format_section_renderer_base::section_edit_controls() can be moved to format_base class. Actually I should have done it in 2.4 when I created format_base class. I only moved is_section_current() at that moment

Average of ratings: -
In reply to Marina Glancy

Re: Changes to course listings in 2.5

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

No, that is not a solution. Do not mix up base classes and helper classes like that.

There should still be an abstract base class format_renderer_base, and all specific renderers like format_topics_renderer should be

class format_topics_renderer extends format_renderer_base {

and use parent:: like normal. Doing anything else leads to insanity.

When you have some common code that needs to be shared between different format renderers, but still overridable, don't put it in format_renderer_base. You need a new renderer

class core_course_commonbits_renderer extends plugin_renderer_base {

format_renderer_base, format_topics_renderer and similar classes will use methods of the course_commonbits_renderer to output shared stuff.

Average of ratings: -
In reply to Tim Hunt

Re: Changes to course listings in 2.5

by Gareth J Barnard -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers

Dear Marina and Tim,

I could be wrong about my assumption as it's how I perceive things at the moment from a functional design point of view.

I agree with Tim with the format_renderer_base still being abstract.

Understanding this from an OO / Java / C++ background perspective I would think that the theme would want to override certain functionality and a course format another.  If you have the structure:

format_renderer_base -> format_renderer_course_format_overrides -> format_renderer_theme_overrides

Because the theme wants to override some of what the course format is doing then you would need a class for each course format you wanted to support unless PHP supports multiple inheritence (nasty).

But as I understand it with what Tim is saying you would have:

plugin_renderer_base -> core_course_commonbits_renderer(abstract) -> format_renderer_base(abstract) -> format_renderer_course_format_overrides

and

plugin_renderer_base -> core_course_commonbits_renderer(abstract) -> theme_renderer_base(abstract) -> theme_renderer_theme_overrides

Such that the theme and the course format can perform overrides.  But what happens when there is a conflict?  Should themes and course formats do the same thing or should there be a distinction in functionality of responsibility?  Such that a theme does specific things and a course format does other specific things and never the twain shall meet.

Cheers,

Gareth

Average of ratings: -
In reply to Gareth J Barnard

Re: Changes to course listings in 2.5

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

That is not what I am saying.

theme_renderer_base does not subclass core_course_commonbits_renderer.

"Prefer aggregation over inheritance."

Average of ratings: -
In reply to Tim Hunt

Re: Changes to course listings in 2.5

by Gareth J Barnard -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers

Dear Tim,

So if both the theme and the course format both aggregate core_course_commonbits_renderer in their abstract base classes how does the theme override any method that the course format does?

Cheers,

Gareth

Average of ratings: -
In reply to Gareth J Barnard

Re: Changes to course listings in 2.5

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

I am wondering what sort of OO / Java / C++ knowledge you acutally have. You are not understanding what I say, and I am not in the mood to explain at greater length now.

Average of ratings: -
In reply to Tim Hunt

Re: Changes to course listings in 2.5

by Gareth J Barnard -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers

Dear Tim,

Perhaps I am not explaining myself, so I have drawn a class digram - attached.

Kind regards,

Gareth

Attachment th.png
Average of ratings: -
In reply to Gareth J Barnard

Re: Changes to course listings in 2.5

by Mary Evans -
Picture of Core developers Picture of Documentation writers Picture of Peer reviewers Picture of Plugin developers Picture of Testers

The only way to keep this discussion from going belly up is to do what Marina says and test the course format renderers, and if they are found to be flawed then file it as a BUG in Moodle Tracker.

I'm too tired to read, and not knowledgeable enough about renderers to comment, although I have discovered a wonderful way to include plugin renderers in themes thanks to Tim, so I guess he knows what he is talking about. smile

Have a nice weekend everyone. Please don't spend it renderers! bah humbug!

Don't worry be happy! cheers

 

 

Average of ratings: Useful (1)
In reply to Gareth J Barnard

Re: Changes to course listings in 2.5

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

Well, they say a picture is worth 1000 words, and your picture certainly qualifies. It is exactly right. Sorry I failed to understand what you were saying before.

Average of ratings: -
In reply to Tim Hunt

Re: Changes to course listings in 2.5

by Gareth J Barnard -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Plugin developers

Dear Tim,

No worries smile - just goes to show why class, object interaction, state transition diagrams... exist to describe the form and function of software.

Cheers,

Gareth

Average of ratings: -