Skip to content
This repository

JLoader : Files located at the root path, where the prefix is registered... #1445

Closed
wants to merge 1 commit into from

2 participants

Florian Voutzinos Rouven Weßling
Florian Voutzinos

No description provided.

Rouven Weßling
Collaborator

I can see how this is useful but I'm afraid of the performance implications. In some testing I did we spend up to 20% of the execution time in the autoloader. If we add more it certainly won't help.

I let other s judge whether it's worth the trade off, but frankly I don't see the big deal with following our folder strcuture. We could put the factory into a factory folder and it would be autoloadable too.

Florian Voutzinos

I added the code at the end, I don't think it changes a lot the current execution time : it adds one variable allocation, and two when there is only one part in the class name.

The if is never executed for platform classes, because there is only one lookup path.

Rouven Weßling
Collaborator

Well it is for all legacy classes. For example in the Joomla CMS we have 3 autoloaders for J prefix so this code would be executed twice in the worst case.

Another thing that just occurred to me is that with this patch we don't have a 1:1 mapping of class names and file paths anymore. Again not a big deal but something to consider (and document).

Edith: The thing is also not the variable but the file_exists() call.

Florian Voutzinos

Yes, in the case of the CMS it can add two file_exists calls in some cases when there is only one part.

What I was thinking also regarding my namespace loader pull request is eventually to have more than one autoloader.

In this case, JLoader would stay like that to auto load the core, and we can have a JLoaderNamespace, and eventually an other Loader with this feature.

It can allow people to use the auto loader they want for their application/components without making the core execution slower, and avoid to register more methods by default to the spl autoloader stack.

Florian Voutzinos florianv closed this October 09, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Aug 10, 2012
Florian Voutzinos JLoader : Files located at the root path, where the prefix is registe…
…red, cannot be autoloaded.
06659f7
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 24 additions and 2 deletions. Show diff stats Hide diff stats

  1. 26  libraries/loader.php
26  libraries/loader.php
@@ -318,8 +318,17 @@ private static function _load($class, $lookup)
318 318
 		// Split the class name into parts separated by camelCase.
319 319
 		$parts = preg_split('/(?<=[a-z0-9])(?=[A-Z])/x', $class);
320 320
 
321  
-		// If there is only one part we want to duplicate that part for generating the path.
322  
-		$parts = (count($parts) === 1) ? array($parts[0], $parts[0]) : $parts;
  321
+		$fileName = false;
  322
+
  323
+		// If there is only one part.
  324
+		if (count($parts) == 1)
  325
+		{
  326
+			// Keep the possible file name.
  327
+			$fileName = $parts[0];
  328
+
  329
+			// Duplicate that part for generating the path.
  330
+			$parts = array($parts[0], $parts[0]);
  331
+		}
323 332
 
324 333
 		foreach ($lookup as $base)
325 334
 		{
@@ -331,6 +340,19 @@ private static function _load($class, $lookup)
331 340
 			{
332 341
 				return include $path;
333 342
 			}
  343
+
  344
+			// If there is only one part.
  345
+			if ($fileName)
  346
+			{
  347
+				// Try to include the class that might be located in the root folder.
  348
+				$path = $base . '/' . strtolower($fileName) . '.php';
  349
+
  350
+				// Load the file if it exists.
  351
+				if (file_exists($path))
  352
+				{
  353
+					return include $path;
  354
+				}
  355
+			}
334 356
 		}
335 357
 	}
336 358
 }
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.