Re: [RFC] Autoloader Classmap

> On Mar 16, 2021, at 1:46 PM, Levi Morrison via internals <internals@lists.php.net> wrote: > > On Tue, Mar 16, 2021 at 11:06 AM Harm Smits <harmsmitsdev@gmail.com> wrote: >> >> Hey, >> >> First time ever replying so hope all went well. >> >> I believe the autoload_set_classmap function's >> signature should change >> rather to something like this: >> >> ``` >> /** >> * Sets the internal autoloader to use the given classmap. >> * >> * @param $mapping key value pair of classes; >> * @param $merge A boolean value indicating whether it should *merge* with >> an already defined classmap or override in its interity; >> * @param $merge_override Whether existing keys in the classmap will be >> overridden. >> * >> * @return void >> */ >> autoload_set_classmap(array $mapping, bool $merge = true, bool >> $merge_override = false): void {} >> ``` >> >> It makes more sense as some projects might have multiple composer projects >> loaded (think about modules in e.g. wordpress), where there can be multiple >> autoloaders. Hence the *$merge* and *$merge_override* variables. I don't >> know whether we should rather use flags for this, but considering named >> arguments are here now, I'd say it makes no sense to use them anymore >> (except for maybe consistency). Not sure about the implementation, but the >> features should be there for sure. >> >> Kind regards, >> Harm Smits > > The proposed API allows users to fetch the current map and set a new > map. In other words, the callers can choose these decisions among > others (merge some keys, override others). I assume what you are proposing as an solution is: $classmap = autoload_get_classmap(); $classmap["Foo"] = "/path/to/Foo.php"; autoload_set_classmap($classmap); Instead of adding a third function to allow this: autoload_add_classmap( [ "Foo" => "/path/to/Foo.php" ]); Did I assume correctly? If yes, then a question (because I do not know enough of about how internal data structure handling works in PHP): Would core not be able to implement an add() more efficiently than a get()/set()? I am especially concerned the get()/set() approach might require allocating memory for the returned array for every get(). If both of those approaches would be equivalent from a memory and/or performance perspective then a get()/set() could be sufficient. But if there is a significant difference in memory use and/or performance then ideally we need an add() (or register()) too, no? -Mike