chrisboulton
Job perform method should be static, and job classes should not be instantiated
php-resque behaves differently to the Ruby equivalent in that it currently instantiated job classes when running jobs(e.g $job = new $jobName(); $job->perform()). It also has setUp and tearDown methods which the Ruby version does not.
The problem is, php-resque does NOT know how to instantiate your classes, which means if you have classes that require arguments to the constructor, you are SOL.
It makes sense for php-resque to statically call the perform method on classes, and then leave it up to your class to then instantiate itself if required. There is also no need for the setUp and tearDown methods as these would be handled in the statically called perform method.
This change is going to break compatibility with all current jobs, and require people to update their code.
Is there a reason why the setUp/tearDown methods are not needed? Guess removing complexity does make sense. I’m not sure if constructing a class makes any difference, since I would prefer to use the Job class as a wrapper and call out to other services inside the Job::perform() method, rather than fuss with code in that class itself.
:+1:
This totally makes sense to me, as we're registering classes as jobs.
The BC break is the big issue here, though.
In my own code i wanted to pull jobs out of a dependency injection container, to do that i added a createInstance event, and used the event to implement my own instance creation. The event could similarly be used to hook in a backwards compatible instantiator, that checks method existance and overrides the fixed implementation if the class hasn't been updated. https://github.com/chrisboulton/php-resque/pull/51
Realizing that this issue is old, I'd still like to weigh in real quick. I tend to agree with @maetl that I prefer to use the Job class as a wrapper for more complex classes, though I take it a step further and say that plugging in a class that isn't specifically designed for use as a Resque Job class is a bad idea in itself. The current functionality differs from that of the Ruby original, but tends to be more in line with the needs of PHP development that way, just as the Ruby version tends to be in line with development paradigms there.
Hey, @chrisboulton - is this still something you'd like to see? We're in the middle of putting in so much backward-compatibility-breaking functionality already that we almost might as well just do this now, unless you've changed your mind in the couple of years since creating this issue.
Actually, it might not be too difficult to keep backward compatibility and still move this direction... We could create an interface (say, Resque_Job_Interface) that specifies just one method (say, public static function perform($job, $args, $queue)) and then check whether the job class implements that interface and proceed accordingly (say, if (is_a($this->payload['class'], 'Resque_Job_Interface', TRUE)) {...} else {...}). Any job implementing the new interface can be called statically, bypassing setUp() and tearDown() and so forth, while jobs that don't implement the interface are instantiated according to the existing model. We would of course deprecate the old approach, then remove it entirely in a year or two.
At any rate, that depends on whether we still want to move that direction. There are advantages either way.