trjhtr

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('JobFactoryJobServiceFactory');
 $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('JobModelJobTable');
 $JobItemTable = $sm->get('JobModelJobItemTable');

 $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('SessionCustomer');
 

 return $this->SessionCustomer;
 

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 SessionCustomer 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(SessionCustomer $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 AurynInjector;
// 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