The code moves almost all the quiz classes into the mod/quiz/classes folder to take advantage of auto-loading.

In order to do this, some classes have had to be re-named, and these are listed in upgrade.txt. Nothing there strikes me as a very big deal.

A more serious problem are the classes that are currently in attemptlib.php. All those class names start 'quiz_' and many of them will be refereed to in renderers, so renaming them will be a big deal. I have not yet decided if it is worth the pain of renaming and moving them now, or whether it is more important to maintain backwards compatibility.

One possibility would be to make a nothing class definition like

abstract class quiz_attempt {}

class mod_quiz_attempt extends quiz_attempt {

    // Existing class definition goes here.

}

I think it needs ot be that way around to make code like

public function review_page(quiz_attempt $attemptobj, ...)

continue to work.

On the other hand, the other existing uses are references to constants like quiz_attempt::FINISHED. The point is that theme designers may well have referenes to these classes, and it would be polite not to break their themes.

I am looking for suggestions from the peer reviewer about this.

Tim,
I take it that you are talking about PHP classes rather that CSS one as these are very different.

I don't know of any themes to date that have quiz specific modifications, other than CSS.

Sorry, yes. I mean PHP classes. The OU themes override these renderers, so I am making pain for myself, but that is OK.

Is Sam Marsall Peer Reviewing this? Otherwise I would if I know what I was doing...it's just that I don't have a clue as to what to look out for.

I found a typo in first line of your changes.

probalby => probably

Just reviewing your changes and wondered if this line is actually correct or not:

-  throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'notyourattempt');

+  throw new mod_quiz_exception($attemptobj->get_quizobj(), 'notyourattempt'); 

I would have thought that this should still stay the same at the beggining only addin mod_ before quiz like so...

- throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'notyourattempt');

+ throw new moodle_mod_quiz_exception($attemptobj->get_quizobj(), 'notyourattempt');

Line 23 of mod/quiz/addrandomform.php → mod/quiz/classes/add_random_form.php
Comment line begins with typo...

* For for 

Hi Tim,

1. I don't like removal of the constants in the settings file QUIZ_SHOWIMAGE_*, that seems pretty ugly to me. How about a constants.php and include it from both (I know thats not really in the autoloading spirit)?
2. This is unrelated to your change and I can't get php to complain about it, but playing with my new toy complained about the use of $this in mod_quiz_access_manager::validate_settings_form_fields. Changing it to $quizform like in the base class would avoid confusion.

Otherwise it all looks good to go!

Thanks Dan.

So, you are not worried about the backwards-compatbility implications of renaming the classes? I think it is worth the pain, but then I am probably biassed.

1. ... I am not sure about a separate contants.php file. That is an extra file to include, so better to move them all to lib.php. Except that, there is a sense in which anything in lib.php is implied to be part of the public API of the quiz. Are some of these constants implementation details? Well, things used in settings are part of the public API. Those values get into backup files, and people need to use the values when creating a quiz using web services. OK, I have convinced myself. -> lib.php.

3. I should probably update the collatorlib references while I am at it. And get_plugin_list_with_file, since there are now more modern alternatives.

> So, you are not worried about the backwards-compatbility implications of renaming the classes? I think it is worth the pain, but then I am probably biassed.

Well - I saw you post about it and I didn't see many objections and really you know the impact better than me, so I didn't really have an opinion Although, surely people modifying quiz itself like this are limited. So it doesn't concern me that much.

I am going to wont-fix this. Rather than a big bang change, I will move classes naturally, when we are doing significant development in that area. E.g. MDL-40987.

$ git grep MDL-33071

mod/quiz/lib.php:    // TODO MDL-33071 it is very annoying to have to included all of locallib.php

Please note that all issues that are closed as resolved or won't fix but still have remaining TODO or @todo in core are linked to MDL-47779 as "causing regressions"