Multiple object instantiation prevention

Question

I have a class with a private variable used to store an object. I have a function that checks first if that variable already contains an object or not; if not, it instantiates the needed object and sets it to that variable, otherwise it just returns the content of that variable.

I was wondering if the getSessionCustomer() here is an overkill/unnecessary or if it has real benefits. I simply based this on the Album tutorial by Zend, but I haven't fully tested it out yet to really see the advantages (or disadvantages).

class JobController extends AbstractActionController
{
 private $SessionCustomer;
 public function saveJobAction()
 {
 $SessionCustomer = $this->getSessionCustomer();
 if(empty($SessionCustomer->offsetGet('customer_id'))) {
 return $this->redirect()->toRoute('login');
 } else {
 $JobService = $this->getServiceLocator()->get('Job\Factory\JobServiceFactory');
 $job_id = $JobService->saveJob();
 return $this->redirect()->toUrl('/job/' . $job_id); 
 }
 }
 public function viewJobAction()
 {
 $sm = $this->getServiceLocator();
 $SessionCustomer = $this->getSessionCustomer();
 if(empty($SessionCustomer->offsetGet('customer_id'))) {
 return $this->redirect()->toRoute('login');
 } else {
 $JobTable = $sm->get('Job\Model\JobTable');
 $JobItemTable = $sm->get('Job\Model\JobItemTable');
 $jobId = $this->params()->fromRoute('id');
 $Job = $JobTable->getJobById($jobId);
 $JobItems = $JobItemTable->getJobItemsByJobId($jobId);
 $this->layout()->setVariable('title', 'Order #' . $jobId);
 $viewModel = new ViewModel();
 $viewModel->setVariables(array(
 'Job' => $Job,
 'JobItems' => $JobItems
 ));
 return $viewModel;
 }
 }
 private function getSessionCustomer()
 {
 if(!$this->SessionCustomer) {
 $this->SessionCustomer = $this->getServiceLocator()->get('Session\Customer');
 }
 return $this->SessionCustomer;
 }
}

Answer 1

Using the check if (!$this->SessionCustomer) is not unusual even if a bit clunky. I would still be more explicit about it and use if ($this->SessionCustomer === null). The private $SessionCustomer; member will be initiated to null.

More generally however, what you are using here is called an anti-pattern. PHP Singletons are almost universally considered an anti-pattern. Singleton really is not something you should be using, as it's really unnecessary, what you really want is Dependency Injection.

When you want to use a single class instance during the lifetime of an object, you simply request the \Session\Customer type in your object's constructor, and use a Dependency Injection Container to make sure that the same instance is passed to all the instances of the JobController class:

class JobController
{
 private $sessionCustomer;
 public function __construct(\Session\Customer $sessionCustomer)
 {
 $this->sessionCustomer = $sessionCustomer;
 }
 public function saveJobAction()
 {
 // just use the private property
 if (empty($this->sessionCustomer->offsetGet('customer_id'))) {
 // do stuff
 }
 }
}

At this point, I would just like to comment that otherwise, there is really no reason to use OOP if you are not using constructor injection and just globally access things with a service locator (which is also a very bad pattern of its own, but that's for another topic). That code could be procedural.

Back to the topic: defining your requirements in the constructor allows for the calling code to decide that instance to pass to the JobController's constructor, reuse that instance, pass in a mock instance (because hopefully, you would be testing that class, which may necessitate for you to mock instances).

I generally use Auryn as a Dependency Injection Container, but the usage generally consists on instructing a container to share an instance of a class:

$injector = new \Auryn\Injector;
// this is how you actually use a single instance of a class, through sharing it with a container
$injector->share($sessionCustomer);
$jobController = $injector->make('JobController'); // uses the shared instance