0
\$\begingroup\$

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;
    }
}
\$\endgroup\$
1
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.