HiveBrain v1.2.0
Get Started
← Back to all entries
patternphplaravelMinor

Eloquent methods to calculate the average rent of apartments

Submitted by: @import:stackexchange-codereview··
0
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?

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 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. amt should be amount, etc.



  • getOneRoomAmtAttribute is 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 (eg attribute), and less focus to the important parts (amt). Something like getAmount($roomCount) would be better.



  • it's hard to understand the difference between avgAmount and avgRent from 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.