I have a controller working like this’
JavaScript
x
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
JavaScript
$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:
JavaScript
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.');
}
}
}
Advertisement
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.
JavaScript
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('...');
}