patternphplaravelMinor
Eloquent methods to calculate the average rent of apartments
Viewed 0 times
theeloquentaveragemethodsrentapartmentscalculate
Problem
I am working on a Laravel 5.1 website where I am counting the average rent of apartments in a specific area. Besides of that I want to get the average rent by amount of rooms in the apartment. The functionality itself isn't anything that hard, but then I want to be able to get the average rent for apartments with e.g. two rooms by a variable.
How would you be able to do this in a cleaner way? I am going to have attributes for up to about 10 rooms, that would mean 20 new methods. Have to be a better way to do this?
How would you be able to do this in a cleaner way? I am going to have attributes for up to about 10 rooms, that would mean 20 new methods. Have to be a better way to do this?
public function average_rent($rooms, $timespan = '6 months', $fromDate = null)
{
if ($fromDate === null) $fromDate = date('Y-m-d');
$query = DB::select(
"SELECT COUNT(id) as amt, SUM(rent) AS total_rent
FROM " . env('DB_PREFIX', '') . "apartments
WHERE
rooms = " . $rooms . " AND rent > 0 AND
formatted_address LIKE '% " . $this->query_params . ", Sverige%' AND
created_at BETWEEN '" . date('Y-m-d', strtotime('-' . $timespan)) . "' AND '" . $fromDate . "'");
$avgRent = null;
$avgAmount = $query[0]->amt;
if ($avgAmount >= 10) $avgRent = round($query[0]->total_rent / $avgAmount, 0) . ' kr';
return [$avgRent, $avgAmount];
}
public function getOneRoomAvgRentAttribute()
{
return $this->average_rent(1)[0];
}
public function getTwoRoomAvgRentAttribute()
{
return $this->average_rent(2)[0];
}
public function getOneRoomAmtAttribute()
{
return $this->average_rent(1)[1];
}
public function getTwoRoomAmtAttribute()
{
return $this->average_rent(2)[1];
}Solution
I am going to have attributes for up to about 10 rooms, that would mean 20 new methods. Have to be a better way to do this?
The only reason you have all these methods in the first place is because it's unclear how to use the
I'm unsure what the difference between amount and rent is, but it may make sense to have two methods:
Security
You are vulnerable to SQL injection. Never put variables into a query directly, always use prepared statements.
Naming
The only reason you have all these methods in the first place is because it's unclear how to use the
average_rent method. If you would just document it properly, you can remove all the other methods that don't actually contain any logic themselves.I'm unsure what the difference between amount and rent is, but it may make sense to have two methods:
getAmount($roomCount) and getAverageRent($roomCount). If both are needed, you could also just query once and save the result in private fields. What you definitely do not want is what you have now, where you add a method for each amount of rooms you expect. That's what method arguments are for.Security
You are vulnerable to SQL injection. Never put variables into a query directly, always use prepared statements.
Naming
- don't shorten variable names, it makes code harder to read.
amtshould beamount, etc.
getOneRoomAmtAttributeis really hard to read. Someone not familiar with your code will have no idea what it will do. It also draws too much focus to irrelevant words (egattribute), and less focus to the important parts (amt). Something likegetAmount($roomCount)would be better.
- it's hard to understand the difference between
avgAmountandavgRentfrom their names. Average amount of what? Money? What's the difference to rent then?
- either always use snake_case or always camelCase, but don't mix them.
Context
StackExchange Code Review Q#119856, answer score: 3
Revisions (0)
No revisions yet.