private or protected

[justinhunt]

Hi Gareth

We are using format_topcoll on a project and its working very well. I wanted to add some icons to the header of each section, but I can't do much by overriding the renderer.

This is because the property variables (e.g $courseformat) are all declared "private" so subclasses can't access them. Is there a good reason why they are not declared "protected," which would allow renderer subclasses to use them?

Thanks

[gjb2048]

Hi Justin,

This is really an OO / functional programming comparison thing. So I've made them 'private' as I take the view that things should be encapsulated as much as possible to avoid being too global and in danger of being altered outside of the scope of their use. Therefore if they were more visible then bugs would be harder to track down, replicate and fix.

So at the time of creation there was no use case that they should be protected. Thus by being 'private' that causes you to ask the question and needing them to be 'protected' for a given purpose, now that purpose can be analysed correctly and all the consequences thereof considered. Thereby thinking about the possible side-effects of the change and putting in place counter measures to reduce them.

So do you need to just 'read' the value or 'read / write'. If the former then a 'constant' getter method might be the better solution. If 'read / write' then need to understand the nature of the problem and possibly create a setter method that changes the value and then ensures the correct state of the renderer object within the behaviour upon which it operates.

In conclusion I need more detail of the changes you need, why and what values etc.

Cheers,

Gareth

[justinhunt]

Thanks for getting back to me. First just let me say, topcoll is working great. Thanks so much for your work on this. And even with the properties declared private, we can get by because we forked it. (https://github.com/ishinegirl/moodle-format_topcoll). I will explain the whole use case. Sorry its a bit looong.

We have a custom section availability condition which we use to enable/disable access to a section based on completion of previous sections. (https://github.com/ishinegirl/moodle-availability_sectioncompleted)

We are hosted with a Moodle partner and can't make changes to core (of course). We can install plugins, including our own, but I am trying to keep custom code to as little as possible so that its easy for someone else to take over.

We want to:

• change the appearance of the section header when the section is unavailable
• remove the ability to toggle the section at all when the section is unavailable
• add a check mark to the section header when all section activities are completed

This can all be done by overriding the renderer class: format_topcoll_renderer , by adding a single class file to the boost/classes/output folder. In that file we just override the section_header() function. So its not a lot of code. And I think its kind of the way the renderer system should work. You just override the renderer without having to fork or hack or get interesting with git.

But right now the overridden function section_header(), even when its a straight copy of the original function, complains because it can't see any of the properties referred to with $this->[property name] in the code. So I think this means you can't really override the topcoll renderer meaningfully without forking.

Whew. Sorry for drawn out explanation. And I hope its nice to hear we are so invested in your work! No hurry on this, and I am happy to push back any changes we make of course.

[gjb2048]

Thanks Justin,

I'll do some thinking.

Kind regards,

Gareth

[gjb2048]

Hi Justin,

Implemented https://github.com/gjb2048/moodle-format_topcoll/commit/4731ef2a4c7b64a97a622134f690ade4872280db in M3.3 version.

Kind regards,

Gareth

[justinhunt]

Awesome, thanks Gareth.