Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move global Krautoload class for PSR-0 compatibility #10

Open
paul-vg opened this issue Jul 15, 2013 · 8 comments
Open

Move global Krautoload class for PSR-0 compatibility #10

paul-vg opened this issue Jul 15, 2013 · 8 comments

Comments

@paul-vg
Copy link
Contributor

paul-vg commented Jul 15, 2013

As of right now, there's no "Vendor Name"[sic] namespace which breaks this mandatory requirement:

  • Each namespace must have a top-level namespace ("Vendor Name").

Preferably, Krautoload shouldn't declare a class in the global namespace. Such is just bad practise.

My suggestion: Move everything to Donquixote\Krautoload.

@donquixote
Copy link
Owner

Hi,
this is going to turn into a bit of discussion..
I am aware you did not invent PSR-0, so this is not against you :)

Truth is... I don't like that part of PSR-0 very much. There is a reason why PSR-4 is going to drop this requirement.
https://github.com/php-fig/fig-standards/blob/master/proposed/autoloader.md

The additional vendor namespace will not add any practical benefit. I don't know of anything out there that would break if some library has a one-level namespace.

On the other hand, it does have some clear disadvantages:

  • It adds one meaningless directory nesting level.
  • It introduces a meaningless author name into the code.
    Or do you remember ever writing $ = johnresig.jQuery ?
  • It means whoever wants to fork it has to change the vendor namespace.

Looking around, I find one library which does the same: Assetic. (and of course every PEAR library, which PSR-0 wants to be backwards-compatible with)

For the \Krautoload::start() in top-level namespace, I know at least one other piece of code which does that - Drupal 8. Now this might not be the perfect example. But it clearly is pretty pretty useful.

@donquixote
Copy link
Owner

So, I might be convinced, but I need some real-world practical arguments. Such as, real-world compatibility issues.

@paul-vg
Copy link
Contributor Author

paul-vg commented Jul 16, 2013

Well, I'll admit I'm not a fan of the FIG either. Think of it as a play-nice-with-others thing. PHP developers are finally creating themselves an ecosystem which revolves around easily deployable and adaptable packages and everything depends on creating a common ground of basic plumbings to build on. The ideas there aren't new, they come from past experiences in projects created and/or maintained by FIG participants. The "vendor" (I still don't like that name no matter how many times I read it) prefix is probably an idea inspired by Java where by convention packages use reverse domain names in order to create unique namespaces for themselves (e.g. com.acme.widgets).

PSR-X replaces the vendor prefix with a base directory to avoid having a set of extra subdirectories for add-on/plugin packages. So for instance, ACME has a namespace for widget adapters ACME\Widget\Adapter and you're writing an optional package for square widgets. You'd have to create all these directories: src/ACME/Widget/Adapter and place Square.php in there. PSR-X addresses this by allowing you to skip the extra directories and define 'src' as the root of your ACME\Widget\Adapter namespace.

Now there are several things going on here:

  • Every class still "MUST begin with a top-level namespace name"
  • The relative class name MUST be mapped to a sub-path by replacing namespace separators with directory separators (emphasis mine)

This means that you still have to namespace everything and _ is no longer mapped to /. For Krautoload, this would require some additional refactoring.

Regardless of what autoloading standard you use, I'd argue that calling the global \Krautoload::start() directly instead of through a use statement is bad practise. Starting with PHP 5.5, there's a class operator which means you never have to use a fully qualified class name again.

EDIT: I've updated the title. The way I see it, the only thing that's missing for PSR-0 compatibility is the requirement that every class must be namespaced. So, that's just one class here.

@donquixote
Copy link
Owner

Well.. I think we both know quite well what PSR-0 and PSR-X are about, so this is mostly an opinion / taste and attitude towards PSRs discussion ..

I personally do care about the standards "on paper", but I also care about the developers who may use this software (who want it to play nice with other PSR-compliant libraries). And if in conflict, I'd rather pick the latter..

Think of it as a play-nice-with-others thing.

Yes, I absolutely support this notion.
The question is, whether we are actually doing anyone a favor by adding an author's github nickname as a meaningless top-level namespace.
I don't know of any PSR-0 related projects that break if one library has any code in a top-level namespace (as long as this namespace is clearly distinguished from other namespaces). After all, PSR-0 was designed to be backwards compatible with PEAR libraries, which totally break this first requirement.

The first requirement of PSR-0 about the vendor namespace is "required" on paper, but noone actually builds any technical assumptions on it.

The theoretical case would be if a project wants to include a PaulVG\Krautoload and a Donquixote\Krautoload at the same time. I personally don't see that as a realistic scenario. You pick either one or the other, or you create your own. Otherwise, Symfony would have to be FabPot\Symfony..

For my taste, the vendor is "the Krautoload community" (which atm is mostly a single person, or the 3 people who are participating in the issue queue). And the package is "the Krautoload class loader". Having namespace Krautoload\Krautoload; would feel quite pointless to me.

Maybe it would make sense to say namespace Krautoload\ClassLoader;, so "ClassLoader" would become a package name. Then another package name could be "ClassDiscovery". But atm I don't see that as being very practical.

@donquixote
Copy link
Owner

This means that you still have to namespace everything and _ is no longer mapped to /. For Krautoload, this would require some additional refactoring.

Yes.. we are not at PSR-4 yet for Krautoload.
Imo, the underscores serve the directory organization quite well for this project. I would not say that for some other projects, where the no-underscore organization is proably the best fit.

On the other hand, no-underscore is not a universal i-win button for nice class names:
https://drupal.org/node/2027221

Going all PSR-4 is not a goal atm. Someone may want to use Krautoload as a library only for class discovery, so it has to work with the class loading shipped with composer.

On the other hand, most other modern PSR-0 projects work underscore-less, so it should be considered to get rid of the underscores, and use namespaces for the directory organization. I am a little afraid this will not make the library nicer to look at, rather the contrary.

@donquixote
Copy link
Owner

I've updated the title. The way I see it, the only thing that's missing for PSR-0 compatibility is the requirement that every class must be namespaced.

See, you are doing it yourself :)
(putting one "mandatory requirement" aside because it sucks)

@donquixote
Copy link
Owner

Regardless of what autoloading standard you use, I'd argue that calling the global \Krautoload::start() directly instead of through a use statement is bad practise.

We could name it like Krautoload\Main::start() or Krautoload\StaticBoot::start() or whatever.
The motivation to just name it Krautoload::start() was to eliminate/avoid arbitrary and meaningless identifier levels.

The idea was also that the class Krautoload itself may not directly participate in PSR-0 or PSR-4 class loading.
If you use Krautoload as a class loader, then you will manually require_once src/Krautoload.php, and then do the bootstrap from there.

So, if you read PSR-0 / PSR-4 as "every class that wants to be class-loaded with PSR-0 / PSR-4 must/should etc", then it would be just fine.

The other thing is naming collisions.. and I don't think that class Krautoload will collide with anything.

Starting with PHP 5.5, there's a class operator which means you never have to use a fully qualified class name again.

Hmm.. what are you refering to?

@donquixote
Copy link
Owner

This being said - if you have a convincing suggestion for Krautoload\Something::start(), I might consider it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants