search

YahnisElsts / wp-update-server

A custom update API for WordPress plugins and themes. Intended to be used in conjunction with my plugin-update-checker library.
MIT License
824 stars 176 forks source link

Implement class autoloading #17

Closed jrfnl closed 10 years ago

jrfnl commented 10 years ago

As the Wp-update-server requires PHP5.3 anyway, we might as well. SPL can't be disabled (in PHP5.3+), so will be available.

YahnisElsts commented 10 years ago

What advantages does autoloading have in the context of this project, aside from making dependencies harder to identify? Given that the number of classes is small (<10), the performance difference is going to be negligible.

jrfnl commented 10 years ago

What advantages does autoloading have in the context of this project

Not much at this moment, but it could have depending on the child implementation. Think downloading a file - you could just check if the file exists and offer it and bypass the Request/Packages/Cache/FileCache classes. I know, it's only four includes, but with remote requests I'm of the opinion that every micro second counts.

aside from making dependencies harder to identify?

I don't see how - could you explain ? The requires were all in one file before, they still are.

jrfnl commented 10 years ago

Also it could make it slightly easier for child implementations to spl_unregister_autoload and implement their own autoload.

YahnisElsts commented 10 years ago

I don't see how - could you explain ? The requires were all in one file before, they still are.

There's more indirection. If someone was unfamiliar with the code, it would probably take them slightly longer to analyse how to autoloader works compared to a handful of requires. Still, for this number of classes it's a minor issue.

Think downloading a file - you could just check if the file exists and offer it and bypass the Request/Packages/Cache/FileCache classes. I know, it's only four includes, but with remote requests I'm of the opinion that every micro second counts.

Hmm, maybe I'll run some tests and will get back to you on this later.

jrfnl commented 10 years ago

I'm not too concerned with this, just though I'd implement it while I was at it anyways. A child implementation would normally overrule the index file anyway, so they could easily include their own loader in favour of this one.

jrfnl commented 10 years ago

Closing for now, it can always be re-opened if you'd reconsider. I won't remove the branch for now.