Well, it won't be 'all classes' in one long directory listing, it will be 'all classes belonging to one plugin' in one directory listing.

Oh, I have just spotted some special cases:

.../backup - this already contains some specific classes in a one-classe-per-file way. Moodle already deals with loading these when necessary. I guess we don't change anything here.

.../tests - similarly, PHPunit loads these when necessary. We can leave this folder alone.

Excluding those, and sub-plugins, the quiz (my guess at the plugin with most classes) contains 25 classes

4 forms

4 admin setting sub-classes (see my complaint in the other thread about the irritation of having to subclass to use lazy-loading of choices)

1 exception type

For me, 25 class files in one folder is not a problem.

Typing a path like mod/quiz/classes/links/to/other/attempts.php is just horrible. It requires (in the best case) "l, Tab, Tab, Tab, Tab". The more sensible mod/quiz/classes/links_to_other_attempts.php requires just (l, Tab). It is worse if the name requires more than the first letter to find. It is also harder to get a list of all classes. With one folder, you just need ls. With deeply nested folders, you need ls -R, or find, or the equivalent in your IDE.

As I said before, I am not just guessing here. This is my experience with YUI modules, which are nice in every way except for the irritating paths they require you to use. I have also found it a pain any time I have had to try to find a particular PEAR or Zend class buried in the lib folder.

Book contains 3 classes. Can anyone find a plugin with more classes than quiz?

The problem we will need to solve is Moodle core. The list of classes there will be too big for one folder. My suggestion there would be:

Where possible, use real components to break the code into logical chunks; and

make a special case for lib/classes (or top-level classes folder if we think it is nicer to keep this outside lib, which is already too big). In that case, we could do one level of nesting, core_xxx_yyy_zzz -> classes/xxx/yyy_zzz.php).

While writing this post I had another possible idea. We could split the classes folder in other ways, for example we could have a separate .../forms folder, at the same level as .../classes, and put classes if there if their name ends _form? We could do something similar for _exception classes in a .../exceptions folder. On the whole, I think we should not do that. It adds complexity, and does not necessarily make it easier to find things.

Actually, I would like the code to be organised into two core components:

core_question - code in question/bank/... - this is the code that deals with what happens when students attempt questions.

core_questionbank - code in question/engine/... - the is the code that deals with what happens when teachers edit questions.

There are a bunch of legacy classes relate to restoring old backup files, and I would not bother to move them to the classes folder.

There are 18 classes in question/engine/states.php that are basically an implementation detail. question_state is that only class there that is part of the public API, so I would leave all these classes in the question/engine/classes/state.php file.

I think that gets the list down to two groups of ~25 classes, which is again manageable.

core will have many classes soon - plugin and core rules should be the same, we are talking hundreds here, not tens

if we had classloading we would have a lot more classes everywhere, looking at current classes does not prove much

proposed events would introduce many classes in all plugins

Tab argumnet is valid for people using shell only, you are forgetting people with IDEs

It is nice of you giving examples of other projects using subdirs, I take it as argument in my favour

question/ is not a good example of code using frankenstyle for core subsystem, it is unique. The proposal for class loading is intended primarily for plugins and lib/classes/*, I am afraid it is too late to enforce strict frankenstyle naming in existing core subsystems.

In theory we could add special classloaders for existing core subsystems that do not use frankenstyle, performance should not be critical factor here.

I think that the most fundamental problem is that it mixes up two completely difference concepts: giving classes good descriptive names, and file paths that group files on file-system in a logical place. For example, mod_quiz_links_to_other_attempts may not be the best name for a class ever, but it does clearly describe what objects of that class are. mod/quiz/classes/links/to/other/attempts.php is a horrible path.

If, right from the start, you know that class-names will be used to group code in folders on disc. Then you can define your class names appropriately. For example Zend_Rest_Server is quite a good name. On the other hand, sometimes you cannot avoid bad names. E.g. HTML_QuickForm_Renderer_Array sounds like an Array of renderers, but it is not.

However, in many parts of Moodle we already have class names that are designed to be good names, with not thought to how that would map to file paths. If we rename the classes now, we break backwards compatibility in many places, just so we can use the same horrible convention as other projects. (I know. It is good to follow the same conventions as everyone else, but sometimes, if a whole lot of lemmings are running off a cliff, you should not join in.)

You mention IDEs. IDEs make the problem worse than command-line. Navigating to deeply nested folders is a real pain there.

Another example is Java. Java groups classes into files according to package names, not class names. This works well, since it lets you have good class names. However, deeply nested package names are still a pain when navigating directory trees. As a result, IDEs have an option to list packages names flattened out, so it is only one click to see the classes in the om.tnavigator.reports.std package with one click, not 4. I don't know of any IDE that does a similar thing for navigating around class files for PHP.

"proposed events would introduce many classes in all plugins" - I have already tried to explain in the other thread why that is a bad idea.

I use IDEs, there is no problem with clicking - we have automatic completion and jump to source...

The spec is not intended for existing core classes and I definitely do not propose renaming of anything.

The class loading spec depends on already established rules for naming of everything in Moodle - Frankenstlye. Martin defined OOP naming rules long time ago, I never liked them, you need to live with them too (all lowercase, same underscores for different things, no namespaces).

Sorry but you did not explain why it is bad idea to put events in classes, I tried to explain real problems in your proposed design and I made a proof of concept that demonstrates the design with code bits, resolves many problems and works consistently

I hope you are right about IDEs. Even at the moment, I sometimes load the file I want to starting to type a class name into a random bit of code, auto-complete, then CTRL + Click. Crazy, but quick!

I am surprised you say that auto-loading should not be for existing code. I have worked hard, in the code I write, to follow best practice. E.g. name classes started mod_quiz_; actually uses classes to organise code; etc. Now you are saying "Because you tried to do the right thing, Tim, you are stewed. You will never be able to benefit from auto-loading." I'm not very happy about that, especially when there are alternative ways to implement auto-loading that work better with existing classes. Oh well.

You are misinterpreting me, the classloading proposal uses consistent existing rules for class names. The classes/ in tinymce is already fully compatible. If anything uses frankentyle rules for plugin class names you could just move the class code to new file in classes/ without renaming or any BC breakage.

I suppose me and you are not going to come to any agreement on classes/ structure. I am going to respect the voting of other core developers.

The core subsystems would have to be probably whitelisted one by one, the rules there should be similar. Most of core subsystems do not use frankenstyle with core_ prefix, introducing it now would break BC badly.

1A is current proposal, 2B seems reasonable too. Mixture of 23BC is another possibility I cosidered and even implemented before (it was a bit slower).

That is a nice summary of the options. I agree with you that C is impossible.

My vote would be 3B. Like you, I would respect the voting of other core developers (more opinions please!). The fundamental point is that auto-loading will be a big win, however we choose to finally implement it. Thank you very much for working on it.

I do think that it is worth having the conversation now to try to find the best way to implement it before we integrate one solution.

How do you feel about classes, rather than lib/classes? lib is already huge, and is a nasty mixture of third-party code and core code. Ah! of course, component core maps to path lib, so the classloader will look in lib/classes, and we don't really want to add an exception for this case.

Maybe we could support fully nested and flat classes dirs at the same time both in core and plugins. That way we could move stuff around as necessary depending on actual number of files there. It would not be that bad for performance.

C is unacceptable because there are just too many different classes in Moodle 'core' that have not been broken down into separate, logical, core subsystems. But that is really a problem with Moodle core, not with the class-loading proposal for modules. Sadly, it is a problem for Moodle core that is essentially unfixable, but I think we should view any exception made for Moodle core to be an inevitable work-around for that. We should not let the problems of Moodle core make things unnecessarily nasty for all other components.

I am afraid I now want to confuse matters by proposing an option 4 / D:

mod_xxx_aaa_bbb_ccc 4/ mod/xxx/classes/cccs/aaa_bbb_ccc.php - 1 level of dirs based on last word.

The reason this is not completely crazy is that we have a lot of classes called something like ..._form, or ..._renderer or ..._exception.

mod/quiz/classes/forms/preflight_check_form.php would be an specific example.

34CD might then be a good choice. I am not sure, and I am not sure how much a choice hurts performance.

I am also not sure that D really solved the problem for Moodle core. But, this might be worth considering.

I would be vaguely in favour of option 2 provided that we are allowed to give classes run-together names in order to ensure they come in the same directory. If we aren't allowed to use run-together names for that reason, then I'd go with option 3.

To put this another way, I have a plugin called format_sciencelab. It has a handful of classes. I want those classes to be in one folder called format/sciencelab/classes; I don't want to have to make a separate level of folders for no reason just for those 5 files.

Here are two example class names:

format_sciencelab_filterformat_sciencelab_searchfilter

As I understand it, either proposal 2 or 3 (& possibly 1?) would result in me being able to put both those classes directly in the classes folder, which is where I have them now. So, yay! And 2 would give the added flexibility of letting me make classes called, I dunno,

format_sciencelab_creature_frogformat_sciencelab_creature_zombie

if I really wanted a 'creature' directory.

So basically, assuming I understood it right I'm down for option 2 if format_sciencelab_searchfilter is allowed as a class name (i.e. if you're allowed to optionally leave out the underline between words in the last bit). If it isn't allowed then I want 3.

One other thing - I am not too bothered about core code with one exception, I think it should go in /classes (/whatever/else) instead of /lib/classes (/whatever/else). The reason is that it's really annoying have Moodle core code dumped in the same place as third-party libraries (just from the point of view of e.g. wanting to exclude third-party stuff from searches, or something like that). This said... this wouldn't really solve the problem because of all existing stuff not being moved... maybe it would be more sensible to move the third-party stuff to a 'thirdparty' root directory? Hmm... OK, better leave this out...

Having read most of what has gone back & forth on this thread, I'm inclined to agree with Tim that I don't like excessive nesting of subdirectories (and I am an IDE user myself - PHPStorm).

Option 2 - mod/xxx/classes/aaa/bbb_ccc.php seems to me to be the best version for plugins - enough nesting to prevent excessive numbers of files in one directory, without the opposite problem of having to navigate through large numbers of subdirectories to find what you are looking for.

I'm not quite sure how that maps onto core libraries (and whether the same solution is needed for both plugins and core code).

I notice that the discussion has also moved on to namespaces, but that is something I can't comment on at all, as I have no experience of working with namespaces ...

I would agree with Petr here but for a different reason because /cc/dd/ee/ff.php is standard in several standard frameworks out there like Symphony, which will make it easier for developers to transition between the two. Making it easier for PHP developers to start developing in Moodle.

This looks great and currently, I'm in favor of the mod_xxx_aaa_bbb_ccc === mod/xxx/classes/aaa/bbb/ccc.php route out of the available options.

I was then thinking about the problem about plugins that have underscores in their name and we could do something like: mod_xxx_aaa_bbb_ccc actually goes to mod/xxx/aaa/bbb/ccc.php. This breaks frankenstyle things, EG: blocks would be blocks_blockname_aaa_bbb_ccc and also plugins that are buried very deeply like grade/grading/form/rubric would have to have ridiculously long class names. So, that idea sucks.

That naturally led me to namespaces. We could do things like component is the namespace and then class names under that component use underscores. So, mod_forum\aaa_bbb_ccc would map to mod/forum/aaa/bbb/ccc.php. Even better, just go full namespaces, mod_forum\aaa\bbb\ccc would map to the same mod/forum/aaa/bbb/ccc.php (Whatever PSR-0 does plus our special sauce to handle frankenstyle). And yes, I saw above that namespaces was a no no, but I'm not really seeing the problem here if we are refactoring class names already to take advantage of autoloading. We should use the best tool for the job and I think namespaces is the tool for this job.

Also, the above allows us to do some potentially faster autoloading because we can identify the component accurately and quickly. From what I have seen, the quickest autoloaders are basically class maps. EG: an array where the key is the class name and the value is the absolute class file path. Doing one array map for all of Moodle would probably be silly and take up too much memory, but one map per component/subsystem that is cached somewhere doesn't seem so silly.

Lastly, you may have noticed that I dropped the classes directory from my examples. The reason is so the autoloader could load files in component/backup, component/db, etc. Unless the intention was to collapse all of those into the classes directory?

After doing some benchmarking, if you find that things are a little sluggish, I recommend investigating the class map idea. Composer somehow generates one automatically, so you might be able to borrow some code there. I suppose though if you just scan a plugin's classes directory, it wouldn't be very challenging to build one.

The problem now is what to do when you want to upgrade your code to take advantage (e.g. MDL-33071) and that requires you to rename your classes to have the proper frankenstyle prefix. For example, suppose you need to rename a class from quiz_old_name extends mod_quiz_new_name. This causes two sorts of code to break:

1. References to the old class name in executable code, e.g.

quiz_old_name::SOME_CONSTANT;

or

$x = new quiz_old_name();

2. References to the old class name in function declarations.

function do_something(quiz_old_name $x) {}

This is most serious when ther is a method in a base class that you are overriding in your subclass.

// This fails with Argument 1 passed to test::method1() must be an instance of test. instance of mod_test given$x->method1($y);

Basically, I am wondering, what is the best way to upgrade my code to work nicely in Moodle 2.6, without breaking backwards-compatibility too much.

Note that it is very easy to search and replace quiz_old_name -> mod_quiz_new_name in any custom code, so perhaps we just need to move forwards, and annoy everyone my making them fix their custom code when they upgrade to 2.6. Having been on the other end of that sort of thing, fixing OU-specific code that was broken by Moodle HQ changes in core, I don't really want to do that.

Having read your post on the Theme's forum and having an interest in OO technology can I ask given that the class 'test' is the temporary additional class, will you ever need to implement 'public function method1(test $x)'? With a parameter type of 'test'.

I can understand that downcasting would not work with an instance of 'mod_test' calling a method only implemented in 'test'. But then in this case you are just mirroring the method names in order to provide instances of the old name 'test' that exist outside of your control the ability to work.