Home controller:
//my query in repository
public function Available (){
$Sql= ' SELECT DISTINCT
a.property_id, a.date, a.minimum_stay,
a.maximum_stay,a.quantity,
p.duration, p.persons, p.amount,
p.extra_person_price, p.minimum_stay AS price_minimum_stay,
p.maximum_stay AS price_maximum_stay, p.weekdays
FROM availabilities AS a
JOIN prices AS p
ON a.property_id=p.property_id
WHERE a.minimum_stay >0
AND a.maximum_stay < 22
AND a.date >= p.period_from
AND a.date <= p.period_till
';
class HomeController extends Controller
{
/**
* @Route("/", name="homepage")
*/
public function indexAction(Request $request)
{
$hotel_information = $this->retrieveInformationFromDB();
return $this->render('Home/index.html.twig', array(
'hotel_information'=>$hotel_information
)
);
}
private function retrieveInformationFromDB(){
$em= $this->getDoctrine()->getEntityManager();
$availables = $em->getRepository('AppBundle:Availabilities')->Available();
$hotel_information=array();
for($i = 0; $i< count($availables) ; $i++) {
$date = $availables[$i]['date'];
$duration = $availables[$i]['duration'];
$amount = $availables[$i]['amount'];
$extra_person_price = $availables[$i]['extra_person_price'];
$persons = explode('|',$availables[$i]['persons']);
$hotel_information_placeholder = $this->calculatePricePerPerson($persons, $extra_person_price, $date, $amount,$duration);
$hotel_information = array_merge($hotel_information, $hotel_information_placeholder);
}
return $hotel_information;
}
private function calculatePricePerPerson($persons, $extra_person_price, $date, $amount,$duration){
foreach ($persons as $person) {
if ($person > 1) {
$price_per_person[] = array(
"date" => $date,
"person" => $person,
"price_person" => number_format((($person * $extra_person_price) + $amount) / 100, 2, '.', ','),
"day2"=>number_format(((($person * $extra_person_price) + $amount)*2) / 100, 2, '.', ','),
"day3"=>number_format(((($person * $extra_person_price) + $amount)*3) / 100, 2, '.', ','),
"day4"=>number_format(((($person * $extra_person_price) + $amount)*4) / 100, 2, '.', ','),
"day5"=>number_format(((($person * $extra_person_price) + $amount)*5) / 100, 2, '.', ','),
"day6"=>number_format(((($person * $extra_person_price) + $amount)*6) / 100, 2, '.', ','),
"day7"=>number_format(((($person * $extra_person_price) + $amount)*7) / 100, 2, '.', ','),
);
} else {
$price_per_person[] = array(
"date" => $date,
"person" => $person,
"price_person" => number_format($amount / 100, 2, '.', ','),
"day2"=>number_format(($amount * 2)/ 100, 2, '.', ','),
"day3"=>number_format(($amount * 3)/ 100, 2, '.', ','),
"day4"=>number_format(($amount * 4)/ 100, 2, '.', ','),
"day5"=>number_format(($amount * 5)/ 100, 2, '.', ','),
"day6"=>number_format(($amount * 6)/ 100, 2, '.', ','),
"day7"=>number_format(($amount * 7)/ 100, 2, '.', ',')
);
}
}
return $price_per_person;
}
In function private function calculatePricePerPerson I am calculating the price for a number of person and number of days based on date. At the moment I have calculated like this. The problem is a calculating number of is from 1 to 21 so therefore my foreach loop get really long and it is really ugly. So My question is how can I do this in better and more efficient way.
This code has several redundant points that can be optimized. The original code :
private function calculatePricePerPerson($persons, $extra_person_price, $date, $amount,$duration){
foreach ($persons as $person) {
if ($person > 1) {
$price_per_person[] = array(
"date" => $date,
"person" => $person,
"price_person" => number_format((($person * $extra_person_price) + $amount) / 100, 2, '.', ','),
"day2"=>number_format(((($person * $extra_person_price) + $amount)*2) / 100, 2, '.', ','),
"day3"=>number_format(((($person * $extra_person_price) + $amount)*3) / 100, 2, '.', ','),
"day4"=>number_format(((($person * $extra_person_price) + $amount)*4) / 100, 2, '.', ','),
"day5"=>number_format(((($person * $extra_person_price) + $amount)*5) / 100, 2, '.', ','),
"day6"=>number_format(((($person * $extra_person_price) + $amount)*6) / 100, 2, '.', ','),
"day7"=>number_format(((($person * $extra_person_price) + $amount)*7) / 100, 2, '.', ','),
);
} else {
$price_per_person[] = array(
"date" => $date,
"person" => $person,
"price_person" => number_format($amount / 100, 2, '.', ','),
"day2"=>number_format(($amount * 2)/ 100, 2, '.', ','),
"day3"=>number_format(($amount * 3)/ 100, 2, '.', ','),
"day4"=>number_format(($amount * 4)/ 100, 2, '.', ','),
"day5"=>number_format(($amount * 5)/ 100, 2, '.', ','),
"day6"=>number_format(($amount * 6)/ 100, 2, '.', ','),
"day7"=>number_format(($amount * 7)/ 100, 2, '.', ',')
);
}
}
return $price_per_person;
}
At first glance, it seems that the codes in both your if and else blocks are quite similar, the only difference being the extra person thing. By focusing on that element, you can avoid having two times almost the same code :
private function calculatePricePerPerson($persons, $extra_person_price, $date, $amount,$duration){
foreach ($persons as $person) {
if ($person > 1) {
$amount += $person * $extra_person_price;
}
$price_per_person[] = array(
"date" => $date,
"person" => $person,
"price_person" => number_format($amount / 100, 2, '.', ','),
"day2"=>number_format(($amount * 2)/ 100, 2, '.', ','),
"day3"=>number_format(($amount * 3)/ 100, 2, '.', ','),
"day4"=>number_format(($amount * 4)/ 100, 2, '.', ','),
"day5"=>number_format(($amount * 5)/ 100, 2, '.', ','),
"day6"=>number_format(($amount * 6)/ 100, 2, '.', ','),
"day7"=>number_format(($amount * 7)/ 100, 2, '.', ',')
);
}
return $price_per_person;
}
Instead of filling the array in each branch of the if, the person price is added to $amount. The else becomes useless and the array is filled all at once since $amount has a different value depending on the case.
Much shorter, but it's not over. The entries day2 to day7 are almost identical, the only difference being the number at the end of the key and factor by which $amount is multiplyed. By chance, they are always the same in each line, so lets reduce it to a loop :
private function calculatePricePerPerson($persons, $extra_person_price, $date, $amount,$duration){
foreach ($persons as $person) {
if ($person > 1) {
$amount += $person * $extra_person_price;
}
$tmp = array(
"date" => $date,
"person" => $person,
"price_person" => number_format($amount / 100, 2, '.', ',')
);
for ($x = 2; $x <= 7; ++$x) {
$tmp["day$x"] = number_format(($amount * $x)/ 100, 2, '.', ',');
}
$price_per_person[] = $tmp;
}
return $price_per_person;
}
During this step, I created a temporary array that is filled with the static data, then a for loop which adds entries to $tmp for each number of day that has to be handled. The temporary array is then assigned to $price_per_person[] so that everything works as it did in the first version. And since the days are now handled buy a loop, you can increase or decrease the amount of days that have to be computed and diplayed simply by modifying the limit in the for - currently 7.
And that's about as short as you can get without losing clarity. Besides, by removing redundancy you can update your code at once instead of - trying to - update every occurence of the - roughly - same code.