Comments (24)
What about creating a autoload.php
file with all require_once()
calls inside?
Example: lib/autoload.php
(untested!)
<?php
// require this file if you aren't using composer, e.g.
// require_once(dirname(__FILE__) . '/php-redmine-api/lib/autoload.php');
require_once(dirname(__FILE__) . '/Redmine/Api/AbstractApi.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Attachment.php');
require_once(dirname(__FILE__) . '/Redmine/Api/CustomField.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Group.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Issue.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueCategory.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssuePriority.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueRelation.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueStatus.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Membership.php');
require_once(dirname(__FILE__) . '/Redmine/Api/News.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Project.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Query.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Role.php');
require_once(dirname(__FILE__) . '/Redmine/Api/SimpleXMLElement.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntry.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntryActivity.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Tracker.php');
require_once(dirname(__FILE__) . '/Redmine/Api/User.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Version.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Wiki.php');
require_once(dirname(__FILE__) . '/Redmine/Client.php');
from php-redmine-api.
@mjorlitzky i'm with @Art4 : could you give his solution a try?
Thanks
from php-redmine-api.
That'll work for me personally, but won't be so nice for our end users if I package it for Gentoo.
- Users who just want to use the library will need to know about the modification we've made. If they just read the documentation, they're going to try to do
require('Redmine/Client.php');
and it won't work. - Any application depending on the library needs to be patched (in how many places?) to run autoload.php before it tries to use the library. These packages might not be maintained by myself, so it's a lot to ask of someone else to patch potentially hundreds of PHP files.
- I have to maintain a patch set on top of whatever the current version(s) are, and update the patch every time a new file is added to the API.
Given those downsides, I'd probably opt to just patch the source files like I've done above. I would still have to update the patch every once in a while, but at least it wouldn't bother anyone else. Of course I would rather do nothing, though =)
from php-redmine-api.
Perhaps too simple-minded, but: Can't you just copy the lib-folder wherever you want it in your project-dir and than, in your project, register the namespace \Redmine to the corresponding dir?
from php-redmine-api.
First of all, copy/pasting code shouldn't be considered a solution to anything =)
The problems it causes are familiar-enough that I don't need to repeat them here, and they're the reason I'd like to be able to create a distro package for the library and install it once globally.
None of the local fixes address the real problem (please excuse the pejorative): the library appears broken for end-users who aren't me. I can't install and maintain a mangled version that requires secret voodoo for our web developers at work, and I can't ship the same to our end-users on Gentoo either. But as is, it just doesn't work.
For example, Api/Attachment.php
requires the AbstractApi
class. It doesn't require('AbstractApi.php');
, though, so the source essentially doesn't "compile." It's missing a dependency. If you're fortunate enough to use Attachment.php
in some other file that has already included AbstractApi.php
, it will appear to work. But it's serendipity in that case: AbstractApi.php
is still missing a dependency.
It's like if you had a Foo
class inherited from Bar
, and you expected everyone who uses Foo
to do require('bar.php');
before they could do require('foo.php')
; it's silly. Composer works around the missing dependency with the nuke-it-from-outer-space approach, but it has its own downsides. It's a bazillion times slower than just specifying the dependencies, since it has to go look around every time you use a class. And it requires an unrelated third-party build tool to be installed in order to run the library, which doesn't make any sense. You don't need GNU Make installed to run cat
, do you?
from php-redmine-api.
@orlitzky if author add require
it will be an overhead. It`s not good. This package optimized for php project. All use composer and this is like standard.
You can simple create bootstrap file or use composer in any system that you want. But composer you will have a lot of benefits.
You don't need GNU Make installed to run cat, do you?
Yes. you don`t But remember there are a lot of contributors to make cat work in all systems *nix. This is other story and this is hard to create php project optimized for web and for command line scripts in any platform.
from php-redmine-api.
So, in your entire project, you don't use ANY classloader (to whom you can register the namespace Redmine, which effectively gives you the required require)?
There is NO chance you can add the three lines from http://php.net/manual/de/language.oop5.autoload.php to some script that uses this api?
from php-redmine-api.
@funivan Have you actually tried it? Autoload is many times slower than just adding the necessary require
directives. Why? Because that's all autoload.php is doing! Except it's doing it on demand, and it doesn't know where the files are.
PHP is not a clever language. When all is said and done, the interpreter sees a big linear blob of code. If the require
directive isn't there, the code won't work. So before the interpreter actually runs Attachment.php
, you can be sure that it's already run AbstractApi.php
. The guy who wrote the code knows where AbstractApi.php
is, so the best you can do is take the unavoidable performance hit of require('AbstractApi.php')
which is not even noticeable. Autoload, on the other hand, has to stop and go look for the file containing the AbstractApi
class every time you use it. Try it before you tell me I'm wrong.
It's not hard to create PHP scripts that work on the web and on the command-line, and PHP is no different than C or any other language. If you do things right, they work. If you don't, they don't. Right now the library crashes out-of-the-box both on the web and the command line. With the proper require
statements, it works on both, and we can package it and make it installable globally and everything is great. It's like 15 lines of require
s that need to be there anyway. Not what I would call hard.
from php-redmine-api.
@osroot25 You're looking at this all wrong. I would like to be able to create a distribution package for php-redmine-api that works out of the box. I don't know who's going to install it, and I can't SSH into their homes and add any number of lines of code to whatever projects they're working on.
I am using this for a personal project, and I am able to work around the problem by patching the source or registering an autoloader or whatever. But I'd like to have the library installed globally and use the package manager to make sure that no one e.g. uninstalls php-redmine-api. To do that, I need to create a package for php-redmine-api. To do that, I need to kill the dependency on composer.
from php-redmine-api.
@orlitzky )) Yes i try it. If you use multiple libraries in project autoloader is best solution. If you use only 1 library and you require all classes of it - possible require is better.
But. I use redmine in several projects. They have multiple libraries with many classes. All of them use autoloader and it is much faster then require all classes of all libraries.
Read it http://blog.ircmaxell.com/2012/07/is-autoloading-good-solution.html
In general: current installation method is very good.
from php-redmine-api.
Loading All The Classes
The first and simplest test loads all 1000 classes. Not surprisingly, the autoload solution is
significantly slower than the hard-coded version. Shockingly over 42% slower. The hard coded test
ran in -on average- 0.0210 seconds. The autoloaded solution ran in -on average- 0.0300 seconds.
So it's clear that if you're loading all of your classes, hard-coding is clearly a better solution.
This is true not only when you "load all your classes," but whenever autoload will load the same number of classes as the manual require
. In this case, I've added the exact require
statements necessary, and autoload.php
would load all of them. So you can expect the manual require
statements to be a whole lot faster.
In your own projects, you don't need to worry either way: I've added the require
statements to the library, so you can remove require('autoload.php');
from your code. You don't, then, have to require
anything else yourself.
from php-redmine-api.
How about add the autoload.php
file to the source and mention it in the docs as an alternative installation method? This will (probably?) solve the issue and keeps the files clean from require
.
The downside is that we have to think about the autoload.php if a new file is added. Or we add a simplified autoloader in autoload.php
.
from php-redmine-api.
@Art4 Is the goal really to keep a few require
statements out of the top of a file? If so, that's probably the best you can do. But:
- It still requires everyone to
require('autoload.php');
in their client code before anything will work. - It's much slower than the manual
require
s - If you ever want to go back and require only what's needed in each file , you'll have to disentangle the mess in
autoload.php
. - It's harder to regenerate
autoload.php
than it is to add a newrequire
at the top of a file.
It would still be better than the current situation, though, where I have no way to generate autoload.php
myself.
from php-redmine-api.
It still requires everyone to require('autoload.php'); in their client code before anything will work.
Sure, but this is the same with your solution. What's the different between require('lib/Redmine/Client.php')
and require('lib/autoload.php')
? How can you get around this require()
?
It's much slower than the manual requires
Hmm, I'm not sure. If you call Client.php
in your solution it will need almost the same time. I'm not sure it will be much slower. We could banchmark this. :-)
If you ever want to go back and require only what's needed in each file , you'll have to disentangle the mess in autoload.php.
I think it will be a greater mess to keep the dependency for each file through require()
calls. If e.g. sometimes Client.php
depends on ClientAbstract.php
you have to remember to add the new require()
call.
And this is a simple untested autoloader:
<?php
// lib/autoload.php
spl_autoload_register(function ($class) {
if (file_exists(dirname(__FILE__) . 'Redmine/Api/' . $class . '.php'))
{
require_once dirname(__FILE__) . 'Redmine/Api/' . $class . '.php';
}
if (file_exists(dirname(__FILE__) . 'Redmine/' . $class . '.php'))
{
require_once dirname(__FILE__) . 'Redmine/' . $class . '.php';
}
});
It's harder to regenerate autoload.php than it is to add a new require at the top of a file.
I don't get this point, sorry. :-/
from php-redmine-api.
Sure, but this is the same with your solution.
I guess if the autoloader is capable of loading the Client
class, it's the same. I was thinking you'd have to do both,
require('lib/autoload.php');
require('lib/Redmine/Client.php');
About the speed, I could try to convince you from first principles that the sky is not pink, but I agree a benchmark is in order. The stupidest thing that could possibly work (benchmark.php
):
<?php
//require('lib/php-redmine-api/lib/autoload.php');
require('lib/php-redmine-api/lib/Redmine/Client.php');
$start_time = microtime(TRUE);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(TRUE);
echo $end_time - $start_time;
?>
At the top, there are two require
statements, one commented out. The first works with your autoload.php
, the second with my patched library. Even if we include the start-up time for the php
process, the numbers I get are waaaayyy in my favor. Averaging 10 trials, I get 0.0006499767303466801s for autoload.php
and 4.37974929809571e-05s for the manual require
statements. That's a 15x slowdown for autoload.php
, and it would be much worse if we didn't start a PHP process each time.
I think it will be a greater mess to keep the dependency for each file through require() calls. If e.g. sometimes Client.php depends on ClientAbstract.php you have to remember to add the new require() call.
You don't have to remember anything. If you add code to e.g. Client.php
that uses some class without the associated require
, then it will crash. So you add the require
. This is how programming without autoload.php
has worked forever =)
And this is a simple untested autoloader
You need to account for the Redmine\
prefix in the class names, but yeah it seems to work. Here's what I wound up with:
<?php
// lib/autoload.php
spl_autoload_register(function ($class) {
$class = str_replace('\\', '/', $class);
$path = dirname(__FILE__) . '/' . $class . '.php';
if (file_exists($path)){
require_once($path);
}
$path = dirname(__FILE__) . '/' . $class . '.php';
if (file_exists($path)) {
require_once($path);
}
});
?>
I don't get this point, sorry. :-/
I thought you were suggesting generating an autoload.php
using composer, and then committing it to the repo. If you write your own, there's no extra work to be done.
I think keeping track of the includes is better for one's long-term sanity, but I would be grateful for this solution as well.
from php-redmine-api.
I should also mention that lib/autoload.php
would be a bad choice of path, in case some other project does the same thing (require('lib/autoload.php');
would then be ambiguous). Something like lib/Redmine/autoload.php
would be less likely to cause problems, but if it comes down to it, the end user could sort that out himself.
from php-redmine-api.
I'm surprised about the benchmark results. I have assumed, that the result would differ for about 50%. The require() calls are the same but in different files. Maybe we should remove the dirname(__FILE__)
in autoload.php.
At the top, there are two require statements, one commented out. The first works with your autoload.php, the second with my patched library.
Are you sure you don't mixed up the code? The benchmark.php looks to me it used the same code. If I'll have time I will test it today. benchmark.php should look like this:
<?php
//require('lib/php-redmine-api_art4/lib/autoload.php');
require('lib/php-redmine-api_orlitzky/lib/Redmine/Client.php');
$start_time = microtime(TRUE);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(TRUE);
echo $end_time - $start_time;
?>
This is how programming without autoload.php has worked forever =)
But this is not how this library worked till today. All developers who have not chanced the code are using the library with composer. If we forget a require()
call composer will catch this and autoload the class.
I recommend to keep the files clear of the require
calls. But regardless of our discussion, the dessision is up to @kbsali.
from php-redmine-api.
I'm surprised about the benchmark results. I have assumed, that the result would differ for about 50%. The require() calls are the same but in different files. Maybe we should remove the
dirname(__FILE__)
in autoload.php.
The difference is much bigger than it looks. If you have two relatively close numbers, like say 5
and 6
, you can say that 6
is 20% bigger. But if you add a big number to both of them -- say, 100
, you get 105
and 106
, and the latter looks not that much bigger anymore. The benchmark I did starts up a php
process each time, and that has the same effect as adding 100
to both numbers: the PHP start-up time is a huge portion of the total runtime. But since it still showed a 15% slowdown, I didn't see a reason to do a more careful benchmark: it's obvious that the autoload is way slower.
Both methods are including the same files, eventually, but autoload has to register a callback, run the callback every time a class is instantiated, check if a file exists, check if it's been required already, etc.
The benchmark doesn't even use any of the features in Client.php
, if you actually do something, autoload will need to load more classes and it will get even slower.
Are you sure you don't mixed up the code? The benchmark.php looks to me it used the same code. If I'll have time I will test it today. benchmark.php should look like this:
Yes, I was simply renaming the php-redmine-api directory.
from php-redmine-api.
But this is not how this library worked till today. All developers who have not chanced the code are using the library with composer. If we forget a require() call composer will catch this and autoload the class.
It occurs to me that, even if you use composer to install the library, nobody is going to require_once 'vendor/autoload.php';
if it isn't necessary. If the require
statements are added to the files that need them, everyone can remove that line from their code. Then no autoloading would occur if you forget to add a new require
statement.
from php-redmine-api.
@orlitzky I have benchmark all three possibilities. 1. my list of require
calls, 2. your solution in the first post, and 3. the simple autoloader. I have used your benchmark.php with some changes.
1. require list
in lib/autoload.php
<?php
// require this file if you aren't using composer, e.g.
// require_once(dirname(__FILE__) . '/php-redmine-api/lib/autoload.php');
require_once(dirname(__FILE__) . '/Redmine/Api/AbstractApi.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Attachment.php');
require_once(dirname(__FILE__) . '/Redmine/Api/CustomField.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Group.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Issue.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueCategory.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssuePriority.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueRelation.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueStatus.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Membership.php');
require_once(dirname(__FILE__) . '/Redmine/Api/News.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Project.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Query.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Role.php');
require_once(dirname(__FILE__) . '/Redmine/Api/SimpleXMLElement.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntry.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntryActivity.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Tracker.php');
require_once(dirname(__FILE__) . '/Redmine/Api/User.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Version.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Wiki.php');
require_once(dirname(__FILE__) . '/Redmine/Client.php');
2. require() on top of files
I have made the changes from your first post.
3. simple autoloader
in lib/autoload.php
<?php
spl_autoload_register(function ($class) {
$class = str_replace('\\', '/', $class);
$path = dirname(__FILE__) . '/' . $class . '.php';
if (file_exists($path)){
require_once($path);
}
});
benchmark.php
I have used this file to benchmark all three possibilities. The results are micro seconds.
<?php
//require('php-redmine-api_art4/lib/autoload.php'); // 1
//require('php-redmine-api_orlitzky/lib/Redmine/Client.php'); // 2
require('php-redmine-api_autoload/lib/autoload.php'); // 3
$start_time = microtime(true);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(true);
echo sprintf('%.10f', $end_time - $start_time) * 100000 . " \n";
Results
Results are in microseconds.
1 (Art4) | 2 (orlitzky) | 3 (Autoloader) |
---|---|---|
4,22001 | 3,50475 | 59,79538 |
4,69685 | 3,98159 | 49,90101 |
5,79357 | 3,60012 | 61,48815 |
3,79086 | 3,91006 | 57,60193 |
4,72069 | 3,50475 | 70,40501 |
6,60419 | 5,19753 | 58,29334 |
3,88622 | 4,91142 | 49,71027 |
5,19753 | 3,31402 | 50,30632 |
4,19617 | 4,00543 | 58,29334 |
5,81741 | 3,38554 | 57,29198 |
3,79086 | 3,60012 | 141,38222 |
4,60148 | 4,41074 | 49,4957 |
4,19617 | 5,6982 | 47,49298 |
5,60284 | 4,41074 | 47,58835 |
4,41074 | 4,29153 | 60,10532 |
3,79086 | 4,1008 | 64,39686 |
4,29153 | 3,38554 | 64,58759 |
4,69685 | 14,49585 | 57,29198 |
3,60012 | 3,60012 | 47,39761 |
3,79086 | 4,1008 | 51,59378 |
Average | Average | Average |
4,5847905 | 4,5704825 | 60,220956 |
tomorrow more...
from php-redmine-api.
I have benchmark all three possibilities.
Thanks for taking the time to do that. Your solution and mine will benchmark roughly the same because right now Client
is the only class that end-users interact with, and Client.php
includes pretty much the entire list of files in the Redmine/Api
folder. But, suppose some day another user-facing class gets added -- let's call it Server.php
since this is all hypothetical and nothing matters =)
If we add ~20 new files under Redmine/Api
called e.g.,
Redmine/Api/ServerAbstractApi.php
Redmine/Api/ServerAttachment.php
...
Redmine/Api/ServerWiki.php
then the two benchmarks would begin to diverge. Ostensibly the Client
class would only need half of the includes, and the Server
class would need the other half. If the end-user just wants to make some client calls, he'd load up Client.php
and pull in half of the API classes with the require
directives at the top of Client.php
. But with all of the require
s in a separate file, he'd have to load everything -- twice as many files as he needs, since he won't be using the server stuff.
Note: I don't actually care about the performance, so that isn't a big deal, but it's part of the reason why I consider require-at-the-top to be a better design. I only started talking benchmarks because it was claimed that autoload was faster and that was given as a reason to leave things alone.
from php-redmine-api.
I've done this benchmark because I thought you benchmarked our first solutions (1 and 2) and I was surprised your solution 2 was 15x faster than mine. 😄 But at all we are talking about microseconds.
To sum everything up:
- Solution 1: fast, keeps the installation methods seperated, but need to be maintained.
- Solution 2: fastest, but need to be maintained
- Solution 3: keeps the installation methods seperated, need not be maintained, but is 15x and more slower than 1 or 2
I think, we discussed everything. @kbsali what do you think?
from php-redmine-api.
@Art4 thanks a lot for the clear benchmark! 👍
As far as I'm concerned, Solution 1 and 3 are the only valid ones; and given that solution 3 appears to be much slower than the 2 others, I would go in favor @Art4 's!
Also, @orlitzky, we have not added any new files/classes in the past year, so maintaining a "static" autoload.php should not be an issue.
Feel free to create a PR to get this in.
Again, thank you all for trying to getting around all this!
from php-redmine-api.
fixed in #96
thanks again @orlitzky !
from php-redmine-api.
Related Issues (20)
- Do not use temporary cache in `Redmine\Api\...::list()` methods
- [Idea] New CommandClient (High-level API), split Client and Configuration
- Rename `Response::getBody()` to `Response::getContent()`
- Add method `UnexpectedResponseException::getResponse()`
- Check Status Code of responses (especially for POST/PUT/DELETE requests)
- Create factory methods in all API classes
- Declare all `Redmine\Api\...` classes as final
- Deprecate `AbstractApi::get()`, `AbstractApi::post()`, `AbstractApi::put()` and `AbstractApi::delete()`
- Wiki page title must be urlencoded
- Third party package support: Could not create XML from array HOT 3
- Fix @deprecated phpdoc syntax
- Deprecate all `\Redmine\Api\Issue::PRIO_*` constants
- Remove `\Redmine\Api\Issue::PRIO_*` constants
- Update to Redmine 5.0.8 and 5.1.2 in tests HOT 1
- Deprecate `getLastResponseStatusCode`, `getLastResponseContentType` and `getLastResponseBody` in Clients
- author_id Params Not Working When Creating New Issue HOT 1
- XML/JSON parameter mismatch in TimeEntry.php HOT 9
- Remove `Redmine\Api\...::listing()` methods
- Deprecate all `Redmine\Api\*::getIdByName()` methods
- Remove all `Redmine\Api\*::getIdByName()` methods
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from php-redmine-api.