0

I have 4 tables :

  • ads (id, name, type_id ,city_id)
  • types (id, slug, name)
  • cities (id, slug, name) (37000 entries)
  • states (id, slug, name) (120 entries)

I can have a url like this : https://example.com/ad,type,city.html or like this : https://example.com/ad,type,state.html

I need to do a search ads filter by slugs and i'm doing like this to get my ads filtered :

$slugs = explode(',', $slugs);
$ads = Ad::where('is_active', true)->get();

foreach ($slug as slug){

    $types = Type::all();
    if ($types->contains('slug', $slug)) {
        $type = $types->first(function ($value) use ($slug) {
        return $value->slug == $slug;
    });

    $ads = $ads->where('type_id', $type->id);

    $cities = City::all();
    if ($cities->contains('slug', $slug)) {
        $city = $cities->first(function ($value) use ($slug) {
        return $city->slug == $slug;
    });

    $ads = $ads->where('city_id', $city->id);

    $states = State::all();
    if ($states->contains('slug', $slug)) {
        $state = $states->first(function ($value) use ($slug) {
        return $state->slug == $slug;
    });

    $ads = $ads->where('state_id', $state->id);

}

return $ads;
}

This works. But as i have many cities it can be slow (between 1 and 2 seconds). How can i improve this ? I was thinking to use cache like memcached to put cities on it, but the result doesn't change so much. Maybe is the way i builded it who is wrong ?

djoo
  • 685
  • 1
  • 7
  • 24
  • 1
    You are not "properly" querying on cities and states - you are getting all these rows(37K is huge for this) and filtering after it on application layer, instead of data layer. You are querying in a for loop too, that's also another performance problem. query with `whereIn` those slugs. – Ersoy Jun 18 '20 at 23:25
  • First: look that you're querying `::all()` in each iteration of loop. Move $types, $cities and $states outside of foreach. – Daniel Jun 18 '20 at 23:31
  • You should delegate the query to the database instead using collections – OsDev Jun 18 '20 at 23:36
  • Or better yet, create one query. WhereHas May come handy. – Vlad Vladimir Hercules Jun 18 '20 at 23:38
  • thanks a lot guys for you advises – djoo Jun 19 '20 at 13:16

1 Answers1

2

1) Loops: Your Type, City & State modals are not gonna change for each loop. It's better to query them before that.

$types = Type::all();
$cities = City::all();
$states = State::all();

foreach ($slug as slug) {

2) Cache: Caching for these three modals seems appropriate.

3) Since I don't want to repeat, please go through this thread on how to optimize. It talks about proper use of select(), Indexing, data types and some more.

Keep me posted in the comments below. Cheers!

Digvijay
  • 7,836
  • 3
  • 32
  • 53
  • thanks a lot mate. Sounds evident about the 1, i don't know why i didn't see this when i wrote it. – djoo Jun 19 '20 at 13:16
  • Bonus. For the cache i configured Redis and did like this : $cities = Cache::get('cities', function () { return DB::table('cities')->get(); }); i save a lot of time ! – djoo Jun 19 '20 at 15:04