(new SitesController)->checkSite($site);

If you're doing this, it probably means that you should have logic that's currently in your controller put into a new class instead. A controller should be about bridging the web transport layer and your domain.

@Marwelln thanks, but this does not work. Using your example I get:

syntax error, unexpected T_OBJECT_OPERATOR

So thought I'd try this but get the same "Trying to get property of non-object" as before:

$SiteController = (new SitesController);
$SiteController->sendEmail('TEST', $site);

@ ShawnMcCool, yes I have been thinking about moving things around, its just the methods I want to call from my QueueController are also being used in the SitesController, and I like to keep related methods together. Although when several controllers use the same ones I sometimes put them in helpers or libraries. I am coming from a Codeigniter background, not sure if thats a reason for my current logic though.

Bit more info, should have explained earlier, but just realised.... if I use the sync queue driver then "artisan queue:work" works OK, but if I switch to Beanstalkd, which is installed and running, then I get the "Trying to get property of non-object" error (have edited the original question to state this).

Show us how you create your queue job.

SitesController:

Queue::push('QueueController@checkSite', array('site' => $site));

QueueController

<?php
class QueueController extends BaseController
{
    public function checkSite($job, $data)
    {
	   $site = $data['site'];
	   $SitesController = (new SitesController);
	   $SitesController->checkSite($site);
    }
}

QueueController is called OK using Beanstalkd, I can replace the above code with a test email for example, and I get the email, it is purely accessing the SitesController controller that gives me issues. Just cant seem to create a SitesController object.

Tried your code, and my original code, all have same error

amityweb said:

@ ShawnMcCool, yes I have been thinking about moving things around, its just the methods I want to call from my QueueController are also being used in the SitesController, and I like to keep related methods together. Although when several controllers use the same ones I sometimes put them in helpers or libraries. I am coming from a Codeigniter background, not sure if thats a reason for my current logic though.

Ideally you'll want to create a new ServiceProvider (which are basically just a sort of formalized "namespace" class which is easy to hook up with Laravel's Facades, so you could simply call Sites::sendEmail), put all your method calls in a class under your app's domain, and tell any of your controllers to call that, instead.

That involves a bit of poking around Laravel's Facades, ServiceProviders and IoC, so if you just want to do it the "PHP" way, you can just move the logic into a vanilla class of your choosing, load it via PSR-0 (or 4) and direct your controllers that way.

The gist is just that you want, as a general rule, for the Controllers to just act as transport methods, responsible only for receiving inputs and giving something back to the user. All other logic should be moved out and delegated to a ServiceProvider, Repository, or anything, really. This gives you much better separation and testability (and the warm fuzzy feeling of clean code)!

thanks @rizqidjamaluddin, I started to read up about Facades and IoC so was considering trying it out. I cant access it statically, and if I change the class to static I get other errors, so wondered if Facades allows me to get around that.

Facades and IoC are really handy. If you have access to Laracasts, the best explanation I've seen is over there. If you're looking for a long-term solution, I highly suggest creating a custom service provider and segregating all those tasty responsibilities!

Before I go down this route, would facades/IoC/Service Provider be the best options? Because it seems long winded to just access a method in another class.

I'd definitely consider them to be the best practice way to do it, since you want to offload the responsibilities. And it's not really hard!

The actual class:

<?php namespace Foo\Sites;

class SitesManager implements SitesManagerInterface {
    public function ping()
    {
        return "foo";
    }
}

Then the service provider:

<?php namespace Foo\Sites;

use Illuminate\Support\ServiceProvider;

class SitesServiceProvider extends ServiceProvider {
    public function register() {
        $this->app->bind("sites", 'Foo\Sites\SiteManager');
    }
}

You don't even technically need to use the service provider; it's just there so you can formally register it, and laravel will call register for you. In a small project you could just stick the $app->bind bit in any ol' file, like the routes file.

Now just add a new line in config/app.php in the providers array. Put in Foo\Sites\SiteServiceProvider.

At this point you can do $app->make['sites'], but that's just ugly, so then you can make it a Facade:

<?php namespace Foo\Facades;

use Illuminate\Support\Facades\Facade;

class Sites extends Facade {
    protected static function getFacadeAccessor()
    {
        return 'sites';
    }
}

And you'll probably also want to add a new alias in the config/app.php file, like 'Sites' => 'Foo\Facades\Sites'. Now you can just use Sites::ping() anywhere in your code and you're good to go!

It might seem like quite a bit of boilerplate, but each bit has its job.

• The separate class is a given.
• The Service Provider is just a grouping for you to bind the class you want. In the future, if you want to change to a different kind of SitesManager class, you can exchange it here. This is also where the actual IoC process takes place.
• The Facade is really just about turning it from an ugly make command into a pseudo-static.

Each of those classes really only have a few words you need to change here and there, so they're not hard after the first time you get them working!

Now you can just use Sites::ping() anywhere in your code and you're good to go!

Sounds good, I shall look into this, thanks a lot. Although I get the impression from these posts facades are not best practice http://philsturgeon.co.uk/blog/2012/12/why-do-some-php-devs-lo... and http://programmingarehard.com/2014/01/11/stop-using-facades.html, but added so we can access a non-static method statically. I love the quote "facades are great for gettin crap done quickly"!

Concerns about using statics are legitimate. They can get really difficult to deal with, especially with concerns like unit testing.

The first link is explained away by laravel not actually using statics; we can work with Facades just fine for testing and swapping things in and out cleanly. That was pretty much the whole goal of IoC, so that's out of the way.

As for the second point, I can't explain it any better than Taylor Otwell himself!

Based on that link... Is Route::get('/', 'HomeController@showWelcome'); already a Facade? Because if so, and I am already using Facades, then it seems silly to not use it for this issue on the basis on not wanting to use facades! Based on the Facade Class reference looks like I am using quite a few http://laravel.com/docs/facades.

Go ahead and search your /vendor/laravel folder to find where all those beloved Form, Route, Input, etc classes extend from. You'd be surprised!

Well, I finally managed to get the Queue to call a method from the controller!! Thanks a lot for your help rizqidjamaluddin! I will probably paste my code here later/soon because its not exactly like any of the help I found online especially as I am referencing a controller method, took a lot of trial and error.