Why can I not return a HTTP response from a private function in symfony?

I have a controller working like this’

public function indexAction() {

    // some stuff cut out

    $openEntries = $this->checkOpenEntries($position);

    if($openEntries === 0) {
        //do some stuff
    }
    elseif ($openEntries === -1) {
    
        //Sends email to notify about the problem
        $body = 'Tried to create position log entry but found more than 1 open log entry for position ' . $position->getName() . ' in ' . $position->getACRGroup()->getName() . ' this should not be possible, there appears to be corrupt data in the database.';
        $this->email($body);
        return new response($body . ' An automatic email has been sent to it@domain.se to notify of the problem, manual inspection is required.');
    
    } else {
        //do some other stuff
    }
}

private function checkOpenEntries($position) {

    //Get all open entries for position
    $repository = $this->getDoctrine()->getRepository('PoslogBundle:Entry');
    $query = $repository->createQueryBuilder('e')
    ->where('e.stop is NULL and e.position = :position')
    ->setParameter('position', $position)
        ->getQuery();
    $results = $query->getResult();     

    if(!isset($results[0])) {
        return 0; //tells caller that there are no open entries
    } else {
        if (count($results) === 1) {
            return $results[0]; //if exactly one open entry, return that object to caller
        } else {
            return -1; //if more than one open entry, null will signify to caller that we have a problem.
        }
    }
}

But I would like to put that response-returning-business already in the private function to clean up my indexAction, but that is not possible, the indexAction will keep going past the point of

$openEntries = $this->checkOpenEntries($position);

even if checkOpenEntries attempts to return a HTTP response.

Ideally I would like it to look like this, is there a way to accomplish that:

public function indexAction() {

    // some stuff cut out

    $openEntries = $this->checkOpenEntries($position);

    if($openEntries === 0) {
        //do some stuff
    } else {
        //do some other stuff
    }
}

private function checkOpenEntries($position) {

    //Get all open entries for position
    $repository = $this->getDoctrine()->getRepository('PoslogBundle:Entry');
    $query = $repository->createQueryBuilder('e')
    ->where('e.stop is NULL and e.position = :position')
    ->setParameter('position', $position)
        ->getQuery();
    $results = $query->getResult();     

    if(!isset($results[0])) {
        return 0; //tells caller that there are no open entries
    } else {
        if (count($results) === 1) {
            return $results[0]; //if exactly one open entry, return that object to caller
        } else {
            //Sends email to notify about the problem
            $body = 'Tried to create position log entry but found more than 1 open log entry for position ' . $position->getName() . ' in ' . $position->getACRGroup()->getName() . ' this should not be possible, there appears to be corrupt data in the database.';
            $this->email($body);
            return new response($body . ' An automatic email has been sent to it@domain.se to notify of the problem, manual inspection is required.');
        }
    }
}

Answer

IMHO, it is not clean the way you mix up the type of method. However, you could do something like this, to make it work.

In addition, I think it would be better if functions and parameters have type, it would be easier to read.

public function indexAction() {

  // some stuff cut out

  $openEntries = $this->checkOpenEntries($position);
  if ($openEntries instanceOf Response) return $openEntries;

  if($openEntries === 0) {
      //do some stuff
  } else {
    //do some other stuff
  }

  // you might also want to add some response here!
  return new Response('...');
}

Advertisement

Answer